Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] pch_can: fix 800k comms issue
From: Wolfgang Grandegger @ 2011-02-09 19:21 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
	qi.wang-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	toshiharu-linux-ECg8zkTtlr0C6LszWs/t0g,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <1297157343-3213-1-git-send-email-tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>

Hi Tomoya,

On 02/08/2011 10:29 AM, Tomoya MORINAGA wrote:
> Currently, 800k comms fails since prop_seg set zero.
> (EG20T PCH CAN register of prop_seg must be set more than 1)
> To prevent prop_seg set to zero, change tseg2_min 1 to 2.
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
>  drivers/net/can/pch_can.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index c42e972..9b171d1 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -187,7 +187,7 @@ static struct can_bittiming_const pch_can_bittiming_const = {
>  	.name = KBUILD_MODNAME,
>  	.tseg1_min = 1,
>  	.tseg1_max = 16,
> -	.tseg2_min = 1,
> +	.tseg2_min = 2,
>  	.tseg2_max = 8,
>  	.sjw_max = 4,
>  	.brp_min = 1,

I just realized that this fix is wrong. You wanted to set tseg1_min to 2
(and not tseg2_min). Could you please send a patch?

Thanks,

Wolfgang.

^ permalink raw reply

* mac addresses of local interfaces do not obey setageing 0
From: Veaceslav Falico @ 2011-02-09 18:17 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, bridge

Hello,

I have a host and a VM inside this host bridged. I've set ageing_time and
forward_delay to 0 and trying to capture all the traffic that goes through that
bridge from my VM, but it fails to capture the traffic that has dst ether
address the same as the hosts address (i.e. I can't capture the traffic to the
host).

>From the code, I see that br->ageing_time doesn't really work with local mac
addresses - has_expired() function never says that a local interface mac address
is expired, because it verifies if fdb->is_static is set and returns right away.

Is this the desired behaviour? If so, is there a way to capture packets with
destination to a local interface from another interface?

I've also done a small patch and it seems to fix the situation, but I am not
sure if it's the right way to do it.


Regards,
Veaceslav

---
 net/bridge/br_fdb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 88485cc..3d380c2 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -61,8 +61,8 @@ static inline unsigned long hold_time(const struct net_bridge *br)
 static inline int has_expired(const struct net_bridge *br,
 				  const struct net_bridge_fdb_entry *fdb)
 {
-	return !fdb->is_static &&
-		time_before_eq(fdb->ageing_timer + hold_time(br), jiffies);
+	return (br->ageing_time == 0) || (!fdb->is_static &&
+		time_before_eq(fdb->ageing_timer + hold_time(br), jiffies));
 }
 
 static inline int br_mac_hash(const unsigned char *mac)

^ permalink raw reply related

* Re: [net-2.6][2.6.38-rc2] panic during stress testing
From: Brandeburg, Jesse @ 2011-02-09 18:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, Pieper, Jeffrey E
In-Reply-To: <20110208.135113.59683468.davem@davemloft.net>



On Tue, 8 Feb 2011, David Miller wrote:

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Date: Tue, 08 Feb 2011 10:32:05 -0800
> 
> > We are currently testing patches and ran into this panic, doesn't
> > immediately seem related to the driver.
> > 
> > During TCP/UDP ipv4/6 stress testing on 82574L, 2.6.38-rc2 x86_64
> > (net-2.6 with e1000e patches under test) gets numerous OOM killer
> > messages, followed by a bug/Oops and panic. could not reproduce the
> > bug/panic on 2.6.37, but the OOM killer messages are still seen.
> > 
> > Could well be related somehow to testing with DEBUG_PAGEALLOC enabled.
> > 
> > panic dump and .config follows:
> > 
> > BUG: unable to handle kernel paging request at ffffffff81089738
> > IP: [<ffffffff812e2139>] dst_destroy+0x4b/0xf3
> > PGD 1695067 PUD 1699063 PMD 10001e1 
> > Oops: 0003 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map
> > CPU 1 
> > Modules linked in: nfsd exportfs lockd sunrpc dca e1000e [last unloaded: igb]
> 
> So some piece of freed memory is being referenced in dst_destroy(), can you
> match dst_destroy+0x4b to a line in that function for your build?

appears to be (best as I can tell)

(gdb) l *(dst_destroy+0x4b)
0x20d is in dst_destroy 
(/usr/src/net-2.6-e1000e/arch/x86/include/asm/atomic.h:123).
118      */
119     static inline int atomic_dec_and_test(atomic_t *v)
120     {
121             unsigned char c;
122
123             asm volatile(LOCK_PREFIX "decl %0; sete %1"
124                          : "+m" (v->counter), "=qm" (c)
125                          : : "memory");
126             return c != 0;
127     }
(gdb) l *(dst_destroy+0x4a)
0x20c is in dst_destroy (net/core/dst.c:235).
230             dst->hh = NULL;
231             if (hh)
232                     hh_cache_put(hh);
233
234             if (neigh) {
235                     dst->neighbour = NULL;
>>> 236                     neigh_release(neigh);
237             }
238
239             dst_entries_add(dst->ops, -1);

283 static inline void neigh_release(struct neighbour *neigh)
284 {
>>> 285         if (atomic_dec_and_test(&neigh->refcnt))
286                 neigh_destroy(neigh);
287 }

neigh appears to be zero? since the check just above in dst_destroy was 
checking it against NULL already, maybe we have a race, with some other 
free of neigh (assigned from dst->neighbor)

This is quickly getting beyond me, I tried to check for some changes 
around dst->neighbor but didn't see anything recent.






^ permalink raw reply

* Re: Linux 2.6.38-rc4 (hysdn: BUG)
From: Randy Dunlap @ 2011-02-09 17:24 UTC (permalink / raw)
  To: Linus Torvalds, netdev; +Cc: Linux Kernel Mailing List, Karsten Keil
In-Reply-To: <AANLkTimmb26UiBSukdNnVdxLJpCGd=QqpCw8vQoHALh-@mail.gmail.com>

on x86_64.  no HYSDN hardware found (correct).
Nearly allmodconfig.


[   65.397577] HYSDN: module Rev: 1.6.6.6 loaded
[   65.397584] HYSDN: network interface Rev: 1.8.6.4 
[   65.398057] HYSDN: 0 card(s) found.
[   65.398121] BUG: unable to handle kernel paging request at ffffffffa06c99f0
[   65.398269] IP: [<ffffffffa06c68ba>] hysdn_getrev+0x2e/0x50 [hysdn]
[   65.398379] PGD 1a14067 PUD 1a18063 PMD 6f6c1067 PTE 800000006ce8c161
[   65.398613] Oops: 0003 [#1] SMP DEBUG_PAGEALLOC
[   65.398805] last sysfs file: /sys/devices/pci0000:00/0000:00:1c.4/0000:03:00.0/irq
[   65.398864] CPU 0 
[   65.398913] Modules linked in: hysdn(+) kernelcapi af_packet nfsd lockd nfs_acl auth_rpcgss exportfs sunrpc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT xt_tcpudp nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables x_tables ipv6 cpufreq_ondemand acpi_cpufreq freq_table mperf binfmt_misc dm_mirror dm_region_hash dm_log dm_multipath scsi_dh dm_mod mousedev joydev evdev mac_hid snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_hwdep snd_seq usbmouse usbkbd snd_seq_device usbhid snd_pcm hid snd_timer 8250_pnp tg3 pcspkr sr_mod rtc_cmos dcdbas sg snd cdrom i2c_i801 rtc_core iTCO_wdt 8250 processor rtc_lib soundcore iTCO_vendor_support serial_core thermal_sys intel_agp snd_page_alloc intel_gtt button hwmon unix ide_pci_gener
 ic ide_core ata_generic pata_acpi ata_piix sd_mod crc_t10dif ext3 jbd mbcache uhci_hcd ohci_hcd ssb mmc_core pcmcia pcmcia_core firmware_class ehci_hcd usbcore nls_base [last unloaded: micr!
 ocode]
[   65.400030] 
[   65.400030] Pid: 2497, comm: modprobe Not tainted 2.6.38-rc4 #1 0TY565/OptiPlex 745                 
[   65.400030] RIP: 0010:[<ffffffffa06c68ba>]  [<ffffffffa06c68ba>] hysdn_getrev+0x2e/0x50 [hysdn]
[   65.400030] RSP: 0018:ffff88006eec1e68  EFLAGS: 00010206
[   65.400030] RAX: ffffffffa06c99f1 RBX: ffffffffa06c99e9 RCX: ffff88007c4159a0
[   65.400030] RDX: 000000000c960c24 RSI: 0000000000000024 RDI: ffffffffa06c99e9
[   65.400030] RBP: ffff88006eec1e78 R08: 0000000000000000 R09: 0000000000000000
[   65.400030] R10: 0000000000000000 R11: ffffffff8124698c R12: ffff88006eec1e88
[   65.400030] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[   65.400030] FS:  00007fe1ae6c76f0(0000) GS:ffff88007c400000(0000) knlGS:0000000000000000
[   65.400030] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   65.400030] CR2: ffffffffa06c99f0 CR3: 000000006eee3000 CR4: 00000000000006f0
[   65.400030] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   65.400030] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   65.400030] Process modprobe (pid: 2497, threadinfo ffff88006eec0000, task ffff88006cf88000)
[   65.400030] Stack:
[   65.400030]  ffff88006eec1e78 0000000000000000 ffff88006eec1eb8 ffffffffa06c3bf8
[   65.400030]  ffff88006cf88000 0000000000000000 0000000000000000 00000000b3346a0d
[   65.400030]  0000000000000000 ffffffffa062b000 ffff88006eec1f18 ffffffffa062b0e2
[   65.400030] Call Trace:
[   65.400030]  [<ffffffffa06c3bf8>] hysdn_procconf_init+0xf5/0x133 [hysdn]
[   65.400030]  [<ffffffffa062b000>] ? hysdn_init+0x0/0x1000 [hysdn]
[   65.400030]  [<ffffffffa062b0e2>] hysdn_init+0xe2/0x1000 [hysdn]
[   65.400030]  [<ffffffff810020e3>] do_one_initcall+0xa9/0x1ef
[   65.400030]  [<ffffffff810d6674>] sys_init_module+0x12b/0x307
[   65.400030]  [<ffffffff8100e942>] system_call_fastpath+0x16/0x1b
[   65.400030] Code: e5 53 48 83 ec 08 0f 1f 44 00 00 be 3a 00 00 00 e8 71 39 c0 e0 48 85 c0 74 1e 48 8d 58 02 be 24 00 00 00 48 89 df e8 5b 39 c0 e0 <c6> 40 ff 00 48 ff 05 db 79 00 00 eb 0e 48 ff 05 da 79 00 00 48 
[   65.400030] RIP  [<ffffffffa06c68ba>] hysdn_getrev+0x2e/0x50 [hysdn]
[   65.400030]  RSP <ffff88006eec1e68>
[   65.400030] CR2: ffffffffa06c99f0
[   65.400030] ---[ end trace bf14fd4acc41f5a9 ]---

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* [PATCH -next] can: softing_cs needs slab.h
From: Randy Dunlap @ 2011-02-09 16:44 UTC (permalink / raw)
  To: Stephen Rothwell, netdev; +Cc: linux-next, LKML, davem, socketcan-core
In-Reply-To: <20110209170225.94eae681.sfr@canb.auug.org.au>

From: Randy Dunlap <randy.dunlap@oracle.com>

softing_cs.c uses kzalloc & kfree, so it needs to include linux/slab.h.

drivers/net/can/softing/softing_cs.c:234: error: implicit declaration of function 'kfree'
drivers/net/can/softing/softing_cs.c:271: error: implicit declaration of function 'kzalloc'

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/net/can/softing/softing_cs.c |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20110209.orig/drivers/net/can/softing/softing_cs.c
+++ linux-next-20110209/drivers/net/can/softing/softing_cs.c
@@ -19,6 +19,7 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/slab.h>
 
 #include <pcmcia/cistpl.h>
 #include <pcmcia/ds.h>

^ permalink raw reply

* Re: linux-next: Tree for February 3 (netfilter)
From: Patrick McHardy @ 2011-02-09 16:09 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, netdev, linux-next, LKML, netfilter-devel,
	Jozsef Kadlecsik
In-Reply-To: <4D52B810.403@oracle.com>

Am 09.02.2011 16:51, schrieb Randy Dunlap:
> On 02/08/11 23:21, Patrick McHardy wrote:
>> Am 04.02.2011 20:15, schrieb Randy Dunlap:
>>> On Thu, 3 Feb 2011 16:00:34 +1100 Stephen Rothwell wrote:
>>>
>>>> Hi all,
>>>>
>>>> Changes since 20110202:
>>>
>>> When SYSCTL and PROC_FS and NETFILTER_NETLINK are not enabled:
>>>
>>> net/built-in.o: In function `try_to_load_type':
>>> ip_set_core.c:(.text+0x3ab49): undefined reference to `nfnl_unlock'
>>> ip_set_core.c:(.text+0x3ab4e): undefined reference to `nfnl_lock'
>>
>> Does this patch fix the problem?
> 
> Yes, it does.  Thanks.
> 
> Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
> 

Thanks, I'll push it upstream with my next batch of patches.

^ permalink raw reply

* Re: linux-next: Tree for February 3 (netfilter)
From: Randy Dunlap @ 2011-02-09 15:51 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Stephen Rothwell, netdev, linux-next, LKML, netfilter-devel,
	Jozsef Kadlecsik
In-Reply-To: <4D524079.1000908@trash.net>

On 02/08/11 23:21, Patrick McHardy wrote:
> Am 04.02.2011 20:15, schrieb Randy Dunlap:
>> On Thu, 3 Feb 2011 16:00:34 +1100 Stephen Rothwell wrote:
>>
>>> Hi all,
>>>
>>> Changes since 20110202:
>>
>> When SYSCTL and PROC_FS and NETFILTER_NETLINK are not enabled:
>>
>> net/built-in.o: In function `try_to_load_type':
>> ip_set_core.c:(.text+0x3ab49): undefined reference to `nfnl_unlock'
>> ip_set_core.c:(.text+0x3ab4e): undefined reference to `nfnl_lock'
> 
> Does this patch fix the problem?

Yes, it does.  Thanks.

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* Re: [PATCH 1/2] atmel/macb: fix device name when SOFT/HARD_IRQ enabled
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-02-09 14:44 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-arm-kernel, netdev, Nicolas Ferre, Paul Chavent
In-Reply-To: <4D529992.7050502@ru.mvista.com>

On 16:41 Wed 09 Feb     , Sergei Shtylyov wrote:
> Hello.
> 
> On 09-02-2011 15:43, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> >From: Paul Chavent<paul.chavent@fnac.net>
> 
> >When listing processes on a system with SOFT/HARD_IRQ enabled,
> >the name of the ethernet device is [irq/eth%d] (instead of [irq/eth0] for example).
> 
> >This patch call the request_irq function after having initialized the name of the device.
> 
> >Signed-off-by: Paul Chavent<paul.chavent@fnac.net>
> >Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> >Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> [...]
> 
> >diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> >index f69e73e..d642e08 100644
> >--- a/drivers/net/macb.c
> >+++ b/drivers/net/macb.c
> [...]
> >@@ -1219,13 +1209,23 @@ static int __init macb_probe(struct platform_device *pdev)
> >  	err = register_netdev(dev);
> >  	if (err) {
> >  		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
> >-		goto err_out_free_irq;
> >+		goto err_out_iounmap;
> >  	}
> >
> >-	if (macb_mii_init(bp) != 0) {
> >+	dev->irq = platform_get_irq(pdev, 0);
> 
>    platform_get_irq() can fail...
request_irq will fail too so do we really need to check it?
> 
> >+	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
> >+			  dev->name, dev);

Best Regards,
J.

^ permalink raw reply

* [PATCH 1/1] netfilter: nf_conntrack: set conntrack templates again if we return NF_REPEAT
From: kaber @ 2011-02-09 14:39 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1297262369-26393-1-git-send-email-kaber@trash.net>

From: Pablo Neira Ayuso <pablo@netfilter.org>

The TCP tracking code has a special case that allows to return
NF_REPEAT if we receive a new SYN packet while in TIME_WAIT state.

In this situation, the TCP tracking code destroys the existing
conntrack to start a new clean session.

[DESTROY] tcp      6 src=192.168.0.2 dst=192.168.1.2 sport=38925 dport=8000 src=192.168.1.2 dst=192.168.1.100 sport=8000 dport=38925 [ASSURED]
    [NEW] tcp      6 120 SYN_SENT src=192.168.0.2 dst=192.168.1.2 sport=38925 dport=8000 [UNREPLIED] src=192.168.1.2 dst=192.168.1.100 sport=8000 dport=38925

However, this is a problem for the iptables' CT target event filtering
which will not work in this case since the conntrack template will not
be there for the new session. To fix this, we reassign the conntrack
template to the packet if we return NF_REPEAT.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/nf_conntrack_core.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index e615119..84f4fcc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -942,8 +942,15 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned int hooknum,
 	if (set_reply && !test_and_set_bit(IPS_SEEN_REPLY_BIT, &ct->status))
 		nf_conntrack_event_cache(IPCT_REPLY, ct);
 out:
-	if (tmpl)
-		nf_ct_put(tmpl);
+	if (tmpl) {
+		/* Special case: we have to repeat this hook, assign the
+		 * template again to this packet. We assume that this packet
+		 * has no conntrack assigned. This is used by nf_ct_tcp. */
+		if (ret == NF_REPEAT)
+			skb->nfct = (struct nf_conntrack *)tmpl;
+		else
+			nf_ct_put(tmpl);
+	}
 
 	return ret;
 }
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 0/1] netfilter: netfilter fixes
From: kaber @ 2011-02-09 14:39 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

Hi Dave,

following is a single netfilter fix for net-2.6.git, fixing use of conntrack
templates when TCP conntrack kills the current conntrack in TIME_WAIT state
in order to have the conntrack core create a fresh one, from Pablo.

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master

Thanks!


^ permalink raw reply

* [PATCH 0/2] netfilter: netfilter update
From: kaber @ 2011-02-09 14:33 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

Hi Dave,

following are two IPVS patches for net-next-2.6, fixing:

- use of an incorrect lock in the SCTP module, from Simon

- a precedence bug in ip_vs_sync_switch_mode(), from Dan Carpenter

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master

Thanks!

^ permalink raw reply

* [PATCH 2/2] IPVS: precedence bug in ip_vs_sync_switch_mode()
From: kaber @ 2011-02-09 14:33 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1297262027-26237-1-git-send-email-kaber@trash.net>

From: Dan Carpenter <error27@gmail.com>

'!' has higher precedence than '&'.  IP_VS_STATE_MASTER is 0x1 so
the original code is equivelent to if (!ipvs->sync_state) ...

Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_sync.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 2a2a836..d1b7298 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -392,7 +392,7 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs->sync_state & IP_VS_STATE_MASTER)
+	if (!(ipvs->sync_state & IP_VS_STATE_MASTER))
 		return;
 	if (mode == ipvs->sysctl_sync_ver || !ipvs->sync_buff)
 		return;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 1/2] IPVS: Use correct lock in SCTP module
From: kaber @ 2011-02-09 14:33 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1297262027-26237-1-git-send-email-kaber@trash.net>

From: Simon Horman <horms@verge.net.au>

Use sctp_app_lock instead of tcp_app_lock in the SCTP protocol module.

This appears to be a typo introduced by the netns changes.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
 net/netfilter/ipvs/ip_vs_proto_sctp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index fb2d04a..b027ccc 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -1101,7 +1101,7 @@ static void __ip_vs_sctp_init(struct net *net, struct ip_vs_proto_data *pd)
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
 	ip_vs_init_hash_table(ipvs->sctp_apps, SCTP_APP_TAB_SIZE);
-	spin_lock_init(&ipvs->tcp_app_lock);
+	spin_lock_init(&ipvs->sctp_app_lock);
 	pd->timeout_table = ip_vs_create_timeout_table((int *)sctp_timeouts,
 							sizeof(sctp_timeouts));
 }
-- 
1.7.2.3


^ permalink raw reply related

* Re: [PATCH 7/8] af_unix: implement poll(POLLOUT) for multicast sockets
From: Alban Crequy @ 2011-02-09 14:14 UTC (permalink / raw)
  To: Alban Crequy
  Cc: David S. Miller, Eric Dumazet, Lennart Poettering, netdev,
	linux-kernel, Ian Molton
In-Reply-To: <1295620788-6002-7-git-send-email-alban.crequy@collabora.co.uk>

Le Fri, 21 Jan 2011 14:39:47 +0000,
Alban Crequy <alban.crequy@collabora.co.uk> a écrit :

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 4147d64..138d9a2 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
...
>  	sock_poll_wait(file, sk_sleep(sk), wait);
                             ^^^^^^^^^^^^
> +#ifdef CONFIG_UNIX_MULTICAST
> +	/*
> +	 * On multicast sockets, we need to check if the receiving queue is
> +	 * full on all peers who don't have UNIX_MREQ_DROP_WHEN_FULL.
> +	 */
> +	if (!other || !unix_sk(other)->mcast_group)
> +		goto skip_multicast;
> +	others = unix_find_multicast_recipients(sk,
> +		unix_sk(other)->mcast_group, &err);
> +	if (!others)
> +		goto skip_multicast;
> +	for (i = others->offset ; i < others->cnt ; i++) {
> +		if (others->items[i].flags & UNIX_MREQ_DROP_WHEN_FULL)
> +			continue;
> +		if (unix_peer(others->items[i].s) != sk) {
> +			sock_poll_wait(file,
> +				&unix_sk(others->items[i].s)->peer_wait, wait);
                                                              ^^^^^^^^^

This code does not work correctly: a poller cannot sleep on two wait
queues at the same time. When the poller is added in ->peer_wait, it
will not be in sk_sleep(sk) so it will miss POLLIN events.

I think I need another wait queue at the group level for waiters of
POLLIN|POLLOUT events. Waiters on that global wait queue would be woken
up when a message is delivered to any peer (in unix_dgram_sendmsg along
sk_data_ready) and when a member of the group receives a message (in
unix_dgram_recvmsg along wake_up_interruptible_sync_poll).

^ permalink raw reply

* Re: [PATCH v2] xen network backend driver
From: Ian Campbell @ 2011-02-09 13:47 UTC (permalink / raw)
  To: Francois Romieu
  Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
	Konrad Rzeszutek Wilk, Ben Hutchings, Herbert Xu
In-Reply-To: <20110208164158.GA9584@electric-eye.fr.zoreil.com>

Thanks for the review. Comments below.

On Tue, 2011-02-08 at 16:41 +0000, Francois Romieu wrote: 
> Ian Campbell <Ian.Campbell@citrix.com> :
> [...]
> >       * Dropped the tasklet mode for the backend worker leaving only the
> >         kthread mode. I will revisit the suggestion to use NAPI on the
> >         driver side in the future, I think it's somewhat orthogonal to
> >         the use of kthread here, but it seems likely to be a worthwhile
> >         improvement either way. 
> 
> I have not dug into bind_interdomain_evtchn_to_irqhandler but I would
> expect the kthread to go away once NAPI is plugged into xenvif_interrupt().

bind_interdomain_evtchn_to_irqhandler is analogous to request_irq except
it takes a foreign domain and an evtchn reference instead of an IRQ so I
think it's use is not related to NAPI vs. kthread.

I figure some better explanation/background for the non-Xen folks
regarding the current structure is probably in order. So:

Netback is effectively implementing a NIC in software. Some of the
operations required to do this are more expensive than what would
normally happen within a driver (e.g. copying to/from guest buffers
posted by the frontend driver). They are operations which would normally
be implemented by hardware/DMA/etc in a non-virtual system.

In some sense the kthread (and netback.c) embodies the "hardware"
portion of netback. The driver portion (interface.c) defers the actual
work to the thread and is mostly a pretty normal driver.

It's possible that switching the driver to NAPI will allow us to pull
some work up out of the netback thread into the NAPI context but I think
the bulk of the work is too expensive to do there. In the past when
netback used tasklets instead of kthreads we found that doing netback
processing in that context had a fairly detrimental effect on the host
(e.g. nothing else gets to run), doing the processing in the kthread
allows it to be scheduled and controlled alongside everything else.

That said I am going to try switching the driver over to NAPI and see if
it is workable to pull some/all of the netback functionality up into
that context but unless it's a blocker for upstream acceptance I would
like to defer that work until afterwards. 

[...]
> > +/*
> > + * Implement our own carrier flag: the network stack's version causes delays
> > + * when the carrier is re-enabled (in particular, dev_activate() may not
> > + * immediately be called, which can cause packet loss; also the etherbridge
> > + * can be rather lazy in activating its port).
> > + */
> 
> I have found a netif_carrier_off(vif->dev) but no
> netif_carrier_on(vif->dev). Did I overlook something ?

Huh, yeah. It's apparently been that way for years. I'll investigate.

[...]
> > +	if (xenvif_schedulable(vif) && !xenvif_queue_full(vif))
> 
> This test appears three times along the code. Factor it out ?

Yes, good idea.

[...]
> > +		netif_wake_queue(vif->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct xenvif *vif = netdev_priv(dev);
> > +
> > +	BUG_ON(skb->dev != dev);
> > +
> > +	if (vif->netbk == NULL)
> 
> How is it supposed to happen ?

Apart from "ifconfig down" it can happen when either the frontend driver
shuts itself down or the toolstack hotunplugs the network device and
tearsdown the backend. 

xenvif_disconnect
	xenvif_down
		xen_netbk_remove_xenvif
			vif->netbk = NULL

However xenvif_down is always called with RTNL held. So perhaps the
check is unnecessary. I'll investigate. 

> 
> > +		goto drop;
> > +
> > +	/* Drop the packet if the target domain has no receive buffers. */
> > +	if (unlikely(!xenvif_schedulable(vif) || xenvif_queue_full(vif)))
> > +		goto drop;
> > +
> > +	/* Reserve ring slots for the worst-case number of fragments. */
> > +	vif->rx_req_cons_peek += xen_netbk_count_skb_slots(vif, skb);
> > +	xenvif_get(vif);
> > +
> > +	if (vif->can_queue && xenvif_queue_full(vif)) {
> > +		vif->rx.sring->req_event = vif->rx_req_cons_peek +
> > +			xenvif_max_required_rx_slots(vif);
> > +		mb(); /* request notification /then/ check & stop the queue */
> > +		if (xenvif_queue_full(vif))
> > +			netif_stop_queue(dev);
> > +	}
> > +
> > +	xen_netbk_queue_tx_skb(vif, skb);
> 
> Why not do the real work (xen_netbk_rx_action) here and avoid the skb list
> lock ?  Batching ?

Partly batching but also for the reasons described above.

> > +
> > +	return 0;
> 
> NETDEV_TX_OK

OK

> > +
> > + drop:
> > +	vif->stats.tx_dropped++;
> > +	dev_kfree_skb(skb);
> > +	return 0;
> 
> NETDEV_TX_OK

There is no NETDEV_TX_DROPPED or similar so I guess this is right?

> > +}
> > +
> [...]
> > +struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
> > +			    unsigned int handle)
> > +{
> > +	int err = 0;
> 
> Useless init.

OK

[...]
> > +	vif = netdev_priv(dev);
> > +	memset(vif, 0, sizeof(*vif));
> 
> Useless memset. It is kzalloced behind the scene.

OK

[...]
> > +	rtnl_lock();
> > +	err = register_netdevice(dev);
> > +	rtnl_unlock();
> 
> register_netdev() will do the locking for you.

OK

[...]
> > +
> > +	struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
> > +	struct gnttab_copy tx_copy_ops[MAX_PENDING_REQS];
> > +
> > +	u16 pending_ring[MAX_PENDING_REQS];
> 
> Group the [MAX_PENDING_REQS] arrays as a single array ?

tx_copy_ops is used to marshal arguments to a hypercall so has to be a
standalone array like that.

The indexes into pending_tx_info and pending_ring are not the same so I
think combining them would be confusing.

> 
> [...]
> > +static int xen_netbk_kthread(void *data)
> > +{
> > +	struct xen_netbk *netbk = (struct xen_netbk *)data;
> 
> Useless cast.

OK

> > +	while (!kthread_should_stop()) {
> > +		wait_event_interruptible(netbk->wq,
> > +				rx_work_todo(netbk)
> > +				|| tx_work_todo(netbk)
> > +				|| kthread_should_stop());
> 
> Please put || at the end of the line.

OK

> [...]
> > +static int __init netback_init(void)
> > +{
> > +	int i;
> > +	int rc = 0;
> > +	int group;
> > +
> > +	if (!xen_pv_domain())
> > +		return -ENODEV;
> > +
> > +	xen_netbk_group_nr = num_online_cpus();
> > +	xen_netbk = vmalloc(sizeof(struct xen_netbk) * xen_netbk_group_nr);
> > +	if (!xen_netbk) {
> > +		printk(KERN_ALERT "%s: out of memory\n", __func__);
> > +		return -ENOMEM;
> > +	}
> > +	memset(xen_netbk, 0, sizeof(struct xen_netbk) * xen_netbk_group_nr);
> 
> vzalloc

OK

[...]
> > +failed_init:
> > +	for (i = 0; i < group; i++) {
> 
> 	while (--group >= 0) ?

Good idea.

> > +		struct xen_netbk *netbk = &xen_netbk[i];
> > +		int j;
> > +		for (j = 0; j < MAX_PENDING_REQS; j++) {
> > +			if (netbk->mmap_pages[i])
>                                               ^ j ?
> > +				__free_page(netbk->mmap_pages[i]);
>                                                               ^ j ?
> > +		}

Yes, good catch, thanks! (actually since --group >= 0 I made this i
throughout) 

Ian.






^ permalink raw reply

* Re: [PATCH 1/2] atmel/macb: fix device name when SOFT/HARD_IRQ enabled
From: Sergei Shtylyov @ 2011-02-09 13:41 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: linux-arm-kernel, netdev, Nicolas Ferre, Paul Chavent
In-Reply-To: <1297255416-6920-1-git-send-email-plagnioj@jcrosoft.com>

Hello.

On 09-02-2011 15:43, Jean-Christophe PLAGNIOL-VILLARD wrote:

> From: Paul Chavent<paul.chavent@fnac.net>

> When listing processes on a system with SOFT/HARD_IRQ enabled,
> the name of the ethernet device is [irq/eth%d] (instead of [irq/eth0] for example).

> This patch call the request_irq function after having initialized the name of the device.

> Signed-off-by: Paul Chavent<paul.chavent@fnac.net>
> Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
[...]

> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index f69e73e..d642e08 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
[...]
> @@ -1219,13 +1209,23 @@ static int __init macb_probe(struct platform_device *pdev)
>   	err = register_netdev(dev);
>   	if (err) {
>   		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
> -		goto err_out_free_irq;
> +		goto err_out_iounmap;
>   	}
>
> -	if (macb_mii_init(bp) != 0) {
> +	dev->irq = platform_get_irq(pdev, 0);

    platform_get_irq() can fail...

> +	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
> +			  dev->name, dev);
> +	if (err) {
> +		printk(KERN_ERR
> +		       "%s: Unable to request IRQ %d (error %d)\n",
> +		       dev->name, dev->irq, err);
>   		goto err_out_unregister_netdev;
>   	}
>
> +	if (macb_mii_init(bp) != 0) {
> +		goto err_out_free_irq;
> +	}

    {} not needed here. I think checkpatch.pl should notice this.

WBR, Sergei

^ permalink raw reply

* [PATCH 1/2] atmel/macb: fix device name when SOFT/HARD_IRQ enabled
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-02-09 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: netdev, Paul Chavent, Nicolas Ferre,
	Jean-Christophe PLAGNIOL-VILLARD

From: Paul Chavent <paul.chavent@fnac.net>

When listing processes on a system with SOFT/HARD_IRQ enabled,
the name of the ethernet device is [irq/eth%d] (instead of [irq/eth0] for example).

This patch call the request_irq function after having initialized the name of the device.

Signed-off-by: Paul Chavent <paul.chavent@fnac.net>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/net/macb.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index f69e73e..d642e08 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -1170,16 +1170,6 @@ static int __init macb_probe(struct platform_device *pdev)
 		goto err_out_disable_clocks;
 	}
 
-	dev->irq = platform_get_irq(pdev, 0);
-	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
-			  dev->name, dev);
-	if (err) {
-		printk(KERN_ERR
-		       "%s: Unable to request IRQ %d (error %d)\n",
-		       dev->name, dev->irq, err);
-		goto err_out_iounmap;
-	}
-
 	dev->netdev_ops = &macb_netdev_ops;
 	netif_napi_add(dev, &bp->napi, macb_poll, 64);
 	dev->ethtool_ops = &macb_ethtool_ops;
@@ -1219,13 +1209,23 @@ static int __init macb_probe(struct platform_device *pdev)
 	err = register_netdev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
-		goto err_out_free_irq;
+		goto err_out_iounmap;
 	}
 
-	if (macb_mii_init(bp) != 0) {
+	dev->irq = platform_get_irq(pdev, 0);
+	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
+			  dev->name, dev);
+	if (err) {
+		printk(KERN_ERR
+		       "%s: Unable to request IRQ %d (error %d)\n",
+		       dev->name, dev->irq, err);
 		goto err_out_unregister_netdev;
 	}
 
+	if (macb_mii_init(bp) != 0) {
+		goto err_out_free_irq;
+	}
+
 	platform_set_drvdata(pdev, dev);
 
 	printk(KERN_INFO "%s: Atmel MACB at 0x%08lx irq %d (%pM)\n",
@@ -1238,10 +1238,10 @@ static int __init macb_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_out_unregister_netdev:
-	unregister_netdev(dev);
 err_out_free_irq:
 	free_irq(dev->irq, dev);
+err_out_unregister_netdev:
+	unregister_netdev(dev);
 err_out_iounmap:
 	iounmap(bp->regs);
 err_out_disable_clocks:
-- 
1.7.2.3


^ permalink raw reply related

* [RFC PATCH net-next] net: rename group sysfs entry to netdev_group
From: Xiaotian feng @ 2011-02-09 10:52 UTC (permalink / raw)
  To: netdev
  Cc: Xiaotian Feng, David S. Miller, Eric Dumazet, Tom Herbert,
	Eric W. Biederman, Stephen Hemminger, Vlad Dogaru

From: Xiaotian Feng <dfeng@redhat.com>

commit a512b92 adds sysfs entry for net device group, but
before this commit, tun also uses group sysfs, so after this
commit checkin, kernel warns like this:
    sysfs: cannot create duplicate filename '/devices/virtual/net/vnet0/group'

Since tun has used this for years, rename sysfs under tun might
break existing userspace, so rename group sysfs entry for net device
group is a better choice.

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tom Herbert <therbert@google.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Vlad Dogaru <ddvlad@rosedu.org>
---
 net/core/net-sysfs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 2e4a393..eb4b5e0 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -295,7 +295,7 @@ static ssize_t show_ifalias(struct device *dev,
 	return ret;
 }
 
-NETDEVICE_SHOW(group, fmt_dec);
+NETDEVICE_SHOW(netdev_group, fmt_dec);
 
 static int change_group(struct net_device *net, unsigned long new_group)
 {
@@ -330,7 +330,7 @@ static struct device_attribute net_class_attributes[] = {
 	__ATTR(flags, S_IRUGO | S_IWUSR, show_flags, store_flags),
 	__ATTR(tx_queue_len, S_IRUGO | S_IWUSR, show_tx_queue_len,
 	       store_tx_queue_len),
-	__ATTR(group, S_IRUGO | S_IWUSR, show_group, store_group),
+	__ATTR(netdev_group, S_IRUGO | S_IWUSR, show_group, store_group),
 	{}
 };
 
-- 
1.7.1


^ permalink raw reply related

* RE: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2011-02-09 10:54 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wolfgang Grandegger

Hi Marc,

Thanks for your review comments.
Please see my comments inline:

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> Sent: Wednesday, February 09, 2011 2:57 PM
> To: Bhupesh SHARMA
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for
> Bosch C_CAN controller
>
> On 02/09/2011 06:03 AM, Bhupesh Sharma wrote:
> > Bosch C_CAN controller is a full-CAN implementation which is
> compliant
> > to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can
> > be obtained from:
> > http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
> > c_can/users_manual_c_can.pdf
> >
> > This patch adds the support for this controller.
> > The following are the design choices made while writing the
> controller
> > driver:
> > 1. Interface Register set IF1 has be used only in the current design.
> > 2. Out of the 32 Message objects available, 16 are kept aside for RX
> >    purposes and the rest for TX purposes.
> > 3. NAPI implementation is such that both the TX and RX paths function
> >    in polling mode.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>
> The driver looks quite good, some comments inline, most of them
> nitpicking and or style related.
>
> Have a look at the netif_stop_queue(). In the at91 driver there are two
> possibilities that to stop the queue. First the next tx mailbox is
> still in use, second we have a wrap around. But your hardware is a bit
> different. Anyways a second look doesn't harm.

As the Tx/Rx path of my driver are based on at91 driver, I have earlier
gone through the possibility of stopping the tx queue in the two cases as
you mentioned above :)

As per at91 specs, MRDY=0 signifies:
"Mailbox data registers cannot be read/written by the software application"

But, after reading the Bosch C_CAN specs Transmission Request Register(TxRqst)'s
bits if set to 1, signify that the transmission of this Message Object is
requested and is not yet done. If you agree we can add a check against the same here.
Please do go through the Bosch C_CAN specs for details:
http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
c_can/users_manual_c_can.pdf

> > ---
> > Changes since V5:
> > 1. Seperated the state change and bus error handling paths.
> > 2. Added logic to write LEC value to 0x7 from CPU to check for
> updates
> >    later.
> > 3. Corrected the ERROR_WARNING handling logic to correctly send error
> >    frames on the bus.
> >
> >  drivers/net/can/Kconfig                |    2 +
> >  drivers/net/can/Makefile               |    1 +
> >  drivers/net/can/c_can/Kconfig          |   15 +
> >  drivers/net/can/c_can/Makefile         |    8 +
> >  drivers/net/can/c_can/c_can.c          |  993
> ++++++++++++++++++++++++++++++++
> >  drivers/net/can/c_can/c_can.h          |  230 ++++++++
> >  drivers/net/can/c_can/c_can_platform.c |  207 +++++++
> >  7 files changed, 1456 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/net/can/c_can/Kconfig  create mode 100644
> > drivers/net/can/c_can/Makefile  create mode 100644
> > drivers/net/can/c_can/c_can.c  create mode 100644
> > drivers/net/can/c_can/c_can.h  create mode 100644
> > drivers/net/can/c_can/c_can_platform.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > 5dec456..1d699e3 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -115,6 +115,8 @@ source "drivers/net/can/mscan/Kconfig"
> >
> >  source "drivers/net/can/sja1000/Kconfig"
> >
> > +source "drivers/net/can/c_can/Kconfig"
> > +
> >  source "drivers/net/can/usb/Kconfig"
> >
> >  source "drivers/net/can/softing/Kconfig"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index
> > 53c82a7..24ebfe8 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -13,6 +13,7 @@ obj-y                             += softing/
> >
> >  obj-$(CONFIG_CAN_SJA1000)  += sja1000/
> >  obj-$(CONFIG_CAN_MSCAN)            += mscan/
> > +obj-$(CONFIG_CAN_C_CAN)            += c_can/
> >  obj-$(CONFIG_CAN_AT91)             += at91_can.o
> >  obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
> >  obj-$(CONFIG_CAN_MCP251X)  += mcp251x.o
> > diff --git a/drivers/net/can/c_can/Kconfig
> > b/drivers/net/can/c_can/Kconfig new file mode 100644 index
> > 0000000..ffb9773
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Kconfig
> > @@ -0,0 +1,15 @@
> > +menuconfig CAN_C_CAN
> > +   tristate "Bosch C_CAN devices"
> > +   depends on CAN_DEV && HAS_IOMEM
> > +
> > +if CAN_C_CAN
> > +
> > +config CAN_C_CAN_PLATFORM
> > +   tristate "Generic Platform Bus based C_CAN driver"
> > +   ---help---
> > +     This driver adds support for the C_CAN chips connected to
> > +     the "platform bus" (Linux abstraction for directly to the
> > +     processor attached devices) which can be found on various
> > +     boards from ST Microelectronics (http://www.st.com)
> > +     like the SPEAr1310 and SPEAr320 evaluation boards.
> > +endif
> > diff --git a/drivers/net/can/c_can/Makefile
> > b/drivers/net/can/c_can/Makefile new file mode 100644 index
> > 0000000..9273f6d
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Makefile
> > @@ -0,0 +1,8 @@
> > +#
> > +#  Makefile for the Bosch C_CAN controller drivers.
> > +#
> > +
> > +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> > +obj-$(CONFIG_CAN_C_CAN_PLATFORM) += c_can_platform.o
> > +
> > +ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> > diff --git a/drivers/net/can/c_can/c_can.c
> > b/drivers/net/can/c_can/c_can.c new file mode 100644 index
> > 0000000..7ef4aa9
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -0,0 +1,993 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * TX and RX NAPI implementation has been borrowed from at91 CAN
> > +driver
> > + * written by:
> > + * Copyright
> > + * (C) 2007 by Hans J. Koch <hjk-vqZO0P4V72/QD6PfKP4TzA@public.gmane.org>
> > + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/if_arp.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/list.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +
> > +#include <linux/can.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +
> > +#include "c_can.h"
> > +
> > +static struct can_bittiming_const c_can_bittiming_const = {
> > +   .name = KBUILD_MODNAME,
> > +   .tseg1_min = 2,         /* Time segment 1 = prop_seg + phase_seg1
> */
> > +   .tseg1_max = 16,
> > +   .tseg2_min = 1,         /* Time segment 2 = phase_seg2 */
> > +   .tseg2_max = 8,
> > +   .sjw_max = 4,
> > +   .brp_min = 1,
> > +   .brp_max = 1024,        /* 6-bit BRP field + 4-bit BRPE field*/
> > +   .brp_inc = 1,
> > +};
> > +
> > +static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg) {
> > +   u32 val = priv->read_reg(priv, reg);
> > +   val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> > +   return val;
> > +}
> > +
> > +static void c_can_enable_all_interrupts(struct c_can_priv *priv,
> > +                                           int enable)
> > +{
> > +   unsigned int cntrl_save = priv->read_reg(priv,
> > +                                           &priv->regs->control);
> > +
> > +   if (enable)
> > +           cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> > +   else
> > +           cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> > +
> > +   priv->write_reg(priv, &priv->regs->control, cntrl_save); }
> > +
> > +static inline int c_can_check_busy_status(struct c_can_priv *priv,
> > +int iface) {
> > +   int count = MIN_TIMEOUT_VALUE;
> > +
> > +   while (count && priv->read_reg(priv,
> > +                           &priv->regs->ifregs[iface].com_req) &
> > +                           IF_COMR_BUSY) {
> > +           count--;
> > +           udelay(1);
> > +   }
> > +
> > +   return count;
>
> it's an unusual return value...maybe return 0 on success and -EBUSY
> otherwise?

Hmm.. this will add the checking MIN_TIMEOUT_VALUE against 0 here,
instead of "c_can_object_get" and "c_can_object_put" routines.
If you persist we can add the same in V7 though.. :)

> > +}
> > +
> > +static inline void c_can_object_get(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +                   IFX_WRITE_LOW_16BIT(objno));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           netdev_err(dev, "timed out in object get\n"); }
> > +
> > +static inline void c_can_object_put(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +                   (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +                   IFX_WRITE_LOW_16BIT(objno));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           netdev_err(dev, "timed out in object put\n"); }
> > +
> > +static void c_can_write_msg_object(struct net_device *dev,
> > +                   int iface, struct can_frame *frame, int objno) {
> > +   int i;
> > +   u16 flags = 0;
> > +   unsigned int id;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (!(frame->can_id & CAN_RTR_FLAG))
> > +           flags |= IF_ARB_TRANSMIT;
> > +
> > +   if (frame->can_id & CAN_EFF_FLAG) {
> > +           id = frame->can_id & CAN_EFF_MASK;
> > +           flags |= IF_ARB_MSGXTD;
> > +   } else
> > +           id = ((frame->can_id & CAN_SFF_MASK) << 18);
> > +
> > +   flags |= IF_ARB_MSGVAL;
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +                           IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, flags |
> > +                           IFX_WRITE_HIGH_16BIT(id));
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           priv->write_reg(priv, &priv->regs->ifregs[iface].data[i /
> 2],
> > +                           frame->data[i] | (frame->data[i + 1] << 8));
> > +   }
> > +
> > +   /* enable interrupt for this message object */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> > +                   (frame->can_dlc & 0xf));
>                                         ^^^^^
>
> the masking should not be needed, as you're using
> can_dropped_invalid_skb()

Ok.

> > +   c_can_object_put(dev, iface, objno, IF_COMM_ALL); }
> > +
> > +static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> > +
> > +}
> > +
> > +static inline void c_can_activate_all_lower_rx_msg_obj(struct
> net_device *dev,
> > +                                           int iface,
> > +                                           int ctrl_mask)
> > +{
> > +   int i;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_MSG_RX_LOW_LAST; i++)
> {
> > +           priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                           ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                                   IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +           c_can_object_put(dev, iface, i, IF_COMM_CONTROL);
> > +   }
> > +}
> > +
> > +static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                           IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL); }
> > +
> > +static void c_can_handle_lost_msg_obj(struct net_device *dev,
> > +                                   int iface, int objno)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   netdev_err(dev, "msg lost in buffer %d\n", objno);
> > +
> > +   c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> ~IF_COMM_TXRQST);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   IF_MCONT_CLR_MSGLST);
> > +
> > +   c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
> > +
> > +   /* create an error msg */
> > +   skb = alloc_can_err_skb(dev, &frame);
> > +   if (unlikely(!skb))
> > +           return;
> > +
> > +   frame->can_id |= CAN_ERR_CRTL;
> > +   frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> > +   stats->rx_errors++;
> > +   stats->rx_over_errors++;
> > +
> > +   netif_receive_skb(skb);
> > +}
> > +
> > +static int c_can_read_msg_object(struct net_device *dev, int iface,
> > +int ctrl) {
> > +   u16 flags, data;
> > +   int i;
> > +   unsigned int val;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   skb = alloc_can_skb(dev, &frame);
> > +   if (!skb) {
> > +           stats->rx_dropped++;
> > +           return -ENOMEM;
> > +   }
> > +
> > +   frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           data = priv->read_reg(priv,
> > +                           &priv->regs->ifregs[iface].data[i / 2]);
> > +           frame->data[i] = data;
> > +           frame->data[i + 1] = data >> 8;
> > +   }
> > +
> > +   flags = priv->read_reg(priv, &priv->regs-
> >ifregs[iface].arb2);
> > +   val = priv->read_reg(priv, &priv->regs->ifregs[iface].arb1) |
> > +           (flags << 16);
> > +
> > +   if (flags & IF_ARB_MSGXTD)
> > +           frame->can_id = (val & CAN_EFF_MASK) | CAN_EFF_FLAG;
> > +   else
> > +           frame->can_id = (val >> 18) & CAN_SFF_MASK;
> > +
> > +   if (flags & IF_ARB_TRANSMIT)
> > +           frame->can_id |= CAN_RTR_FLAG;
>
> Can you please only copy the data to the frame if the rtr flag isn't
> set. Not all driver do this yet, but we've agreed to do so.

Ok.

> > +
> > +   netif_receive_skb(skb);
> > +
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += frame->can_dlc;
> > +
> > +   return 0;
> > +}
> > +
> > +static void c_can_setup_receive_object(struct net_device *dev, int
> iface,
> > +                                   int objno, unsigned int mask,
> > +                                   unsigned int id, unsigned int mcont) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].mask1,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].mask2,
> > +                   IFX_WRITE_HIGH_16BIT(mask));
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +                   IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2,
> > +                   (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> mcont);
> > +   c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> ~IF_COMM_TXRQST);
> > +
> > +   netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1)); }
> > +
> > +static void c_can_inval_msg_object(struct net_device *dev, int
> iface,
> > +int objno) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1, 0);
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, 0);
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, 0);
> > +
> > +   c_can_object_put(dev, iface, objno, IF_COMM_ARB |
> IF_COMM_CONTROL);
> > +
> > +   netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1)); }
> > +
> > +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> > +                                   struct net_device *dev)
> > +{
> > +   u32 msg_obj_no;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct can_frame *frame = (struct can_frame *)skb->data;
> > +
> > +   if (can_dropped_invalid_skb(dev, skb))
> > +           return NETDEV_TX_OK;
> > +
> > +   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);
> > +   can_put_echo_skb(skb, dev, msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> > +
> > +   priv->tx_next++;
> > +   if ((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) == 0)
> > +           netif_stop_queue(dev);
>
> It it possible, that the "tx_next" object isn't available, yet?
> Understanding your code right. It's not possible, because you have a
> wrap around (which is handled above) first.

Please see explanation above.

> > +
> > +   return NETDEV_TX_OK;
> > +}
> > +
> > +static int c_can_set_bittiming(struct net_device *dev) {
> > +   unsigned int reg_btr, reg_brpe, ctrl_save;
> > +   u8 brp, brpe, sjw, tseg1, tseg2;
> > +   u32 ten_bit_brp;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   const struct can_bittiming *bt = &priv->can.bittiming;
> > +
> > +   /* c_can provides a 6-bit brp and 4-bit brpe fields */
> > +   ten_bit_brp = bt->brp - 1;
> > +   brp = ten_bit_brp & BTR_BRP_MASK;
> > +   brpe = ten_bit_brp >> 6;
> > +
> > +   sjw = bt->sjw - 1;
> > +   tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> > +   tseg2 = bt->phase_seg2 - 1;
> > +   reg_btr = brp | (sjw << BTR_SJW_SHIFT) | (tseg1 <<
> BTR_TSEG1_SHIFT) |
> > +                   (tseg2 << BTR_TSEG2_SHIFT);
> > +   reg_brpe = brpe & BRP_EXT_BRPE_MASK;
> > +
> > +   netdev_info(dev,
> > +           "setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
> > +
> > +   ctrl_save = priv->read_reg(priv, &priv->regs->control);
> > +   priv->write_reg(priv, &priv->regs->control,
> > +                   ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > +   priv->write_reg(priv, &priv->regs->btr, reg_btr);
> > +   priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
> > +   priv->write_reg(priv, &priv->regs->control, ctrl_save);
> > +
> > +   return 0;
> > +}
> > +
> > +/*
> > + * Configure C_CAN message objects for Tx and Rx purposes:
> > + * C_CAN provides a total of 32 message objects that can be
> > +configured
> > + * either for Tx or Rx purposes. Here the first 16 message objects
> > +are used as
> > + * a reception FIFO. The end of reception FIFO is signified by the
> > +EoB bit
> > + * being SET. The remaining 16 message objects are kept aside for Tx
> purposes.
> > + * See user guide document for further details on configuring
> message
> > + * objects.
> > + */
> > +static void c_can_configure_msg_objects(struct net_device *dev) {
> > +   int i;
> > +
> > +   /* first invalidate all message objects */
> > +   for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_NO_OF_OBJECTS; i++)
> > +           c_can_inval_msg_object(dev, 0, i);
> > +
> > +   /* 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); }
> > +
> > +/*
> > + * Configure C_CAN chip:
> > + * - enable/disable auto-retransmission
> > + * - set operating mode
> > + * - configure message objects
> > + */
> > +static void c_can_chip_config(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> > +           /* disable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_DISABLE_AR);
> > +   else
> > +           /* enable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_ENABLE_AR);
> > +
> > +   if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> > +                                   CAN_CTRLMODE_LOOPBACK)) {
> > +           /* loopback + silent mode : useful for hot self-test */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test,
> > +                           TEST_LBACK | TEST_SILENT);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > +           /* loopback mode : useful for self-test function */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> > +           /* silent mode : bus-monitoring mode */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
> > +   } else
> > +           /* normal mode*/
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> > +
> > +   /* configure message objects */
> > +   c_can_configure_msg_objects(dev);
> > +
> > +   /* set a `lec` value so that we can check for updates later */
> > +   priv->write_reg(priv, &priv->regs->status, LEC_UNUSED); }
> > +
> > +static void c_can_start(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* enable status change, error and module interrupts */
> > +   c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +
> > +   /* basic c_can configuration */
> > +   c_can_chip_config(dev);
> > +
> > +   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > +   /* reset tx helper pointers */
> > +   priv->tx_next = priv->tx_echo = 0;
> > +}
> > +
> > +static void c_can_stop(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   /* set the state as STOPPED */
> > +   priv->can.state = CAN_STATE_STOPPED; }
> > +
> > +static int c_can_set_mode(struct net_device *dev, enum can_mode
> mode)
> > +{
> > +   switch (mode) {
> > +   case CAN_MODE_START:
> > +           c_can_start(dev);
> > +           netif_wake_queue(dev);
> > +           break;
> > +   default:
> > +           return -EOPNOTSUPP;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int c_can_get_berr_counter(const struct net_device *dev,
> > +                                   struct can_berr_counter *bec)
> > +{
> > +   unsigned int reg_err_counter;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +   bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
> > +                           ERR_CNT_REC_SHIFT;
> > +   bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
> > +
> > +   return 0;
> > +}
> > +
> > +/*
> > + * theory of operation:
> > + *
> > + * priv->tx_echo holds the number of the oldest can_frame put for
> > + * transmission into the hardware, but not yet ACKed by the CAN tx
> > + * complete IRQ.
> > + *
> > + * We iterate from priv->tx_echo to priv->tx_next and check if the
> > + * packet has been transmitted, echo it back to the CAN framework.
> > + * If we discover a not yet transmitted package, stop looking for
> more.
> > + */
> > +static void c_can_do_tx(struct net_device *dev) {
> > +   u32 val;
> > +   u32 msg_obj_no;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +
> > +   for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv-
> >tx_echo++) {
> > +           msg_obj_no = get_tx_echo_msg_obj(priv);
> > +           c_can_inval_msg_object(dev, 0, msg_obj_no);
> > +           val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> > +           if (!(val & (1 << msg_obj_no))) {
> > +                   can_get_echo_skb(dev,
> > +                                   msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> > +                   stats->tx_bytes += priv->read_reg(priv,
> > +                                   &priv->regs->ifregs[0].msg_cntrl)
> > +                                   & IF_MCONT_DLC_MASK;
> > +                   stats->tx_packets++;
> > +           }
> > +   }
> > +
> > +   /* restart queue if wrap-up or if queue stalled on last pkt */
> > +   if (((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) != 0) ||
> > +                   ((priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) == 0))
> > +           netif_wake_queue(dev);
> > +}
> > +
> > +/*
> > + * theory of operation:
> > + *
> > + * c_can core saves a received CAN message into the first free
> > +message
> > + * object it finds free (starting with the lowest). Bits NEWDAT and
> > + * INTPND are set for this message object indicating that a new
> > +message
> > + * has arrived. To work-around this issue, we keep two groups of
> > +message
> > + * objects whose partitioning is defined by C_CAN_MSG_OBJ_RX_SPLIT.
> > + *
> > + * To ensure in-order frame reception we use the following
> > + * approach while re-activating a message object to receive further
> > + * frames:
> > + * - if the current message object number is lower than
> > + *   C_CAN_MSG_RX_LOW_LAST, do not clear the NEWDAT bit while
> clearing
> > + *   the INTPND bit.
> > + * - if the current message object number is equal to
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of all lower
> > + *   receive message objects.
> > + * - if the current message object number is greater than
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of
> > + *   only this message object.
> > + */
> > +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, &priv->regs->intpnd1);
> > +
> > +   for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> > +                   msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
> > +                   val = c_can_read_reg32(priv, &priv->regs->intpnd1),
> > +                   msg_obj++) {
> > +           /*
> > +            * as interrupt pending register's bit n-1 corresponds to
> > +            * message object n, we need to handle the same properly.
> > +            */
> > +           if (val & (1 << (msg_obj - 1))) {
> > +                   c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
> > +                                   ~IF_COMM_TXRQST);
> > +                   msg_ctrl_save = priv->read_reg(priv,
> > +                                   &priv->regs->ifregs[0].msg_cntrl);
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_EOB)
> > +                           return num_rx_pkts;
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_MSGLST) {
> > +                           c_can_handle_lost_msg_obj(dev, 0, msg_obj);
> > +                           num_rx_pkts++;
> > +                           quota--;
> > +                           continue;
> > +                   }
> > +
> > +                   if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
> > +                           continue;
> > +
> > +                   /* read the data from the message object */
> > +                   c_can_read_msg_object(dev, 0, msg_ctrl_save);
> > +
> > +                   if (msg_obj < C_CAN_MSG_RX_LOW_LAST)
> > +                           c_can_mark_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj > C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate this msg obj */
> > +                           c_can_activate_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj == C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate all lower message objects */
> > +                           c_can_activate_all_lower_rx_msg_obj(dev,
> > +                                           0, msg_ctrl_save);
> > +
> > +                   num_rx_pkts++;
> > +                   quota--;
> > +           }
> > +   }
> > +
> > +   return num_rx_pkts;
> > +}
> > +
> > +static inline int c_can_has_and_handle_berr(struct c_can_priv *priv)
> > +{
> > +   return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> > +           (priv->current_status & LEC_UNUSED); }
> > +
> > +static int c_can_handle_state_change(struct net_device *dev,
> > +                           enum c_can_bus_error_types error_type) {
> > +   unsigned int reg_err_counter;
> > +   unsigned int rx_err_passive;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct can_frame *cf;
> > +   struct sk_buff *skb;
> > +   struct can_berr_counter bec;
> > +
> > +   /* propogate the error condition to the CAN stack */
> > +   skb = alloc_can_err_skb(dev, &cf);
> > +   if (unlikely(!skb))
> > +           return 0;
> > +
> > +   c_can_get_berr_counter(dev, &bec);
> > +   reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +   rx_err_passive = (reg_err_counter & ERR_CNT_RP_MASK) >>
> > +                           ERR_CNT_RP_SHIFT;
> > +
> > +   switch (error_type) {
> > +   case C_CAN_ERROR_WARNING:
> > +           /* error warning state */
> > +           priv->can.can_stats.error_warning++;
> > +           priv->can.state = CAN_STATE_ERROR_WARNING;
> > +           cf->can_id |= CAN_ERR_CRTL;
> > +           cf->data[1] = (bec.txerr > bec.rxerr) ?
> > +                   CAN_ERR_CRTL_TX_WARNING :
> > +                   CAN_ERR_CRTL_RX_WARNING;
> > +           cf->data[6] = bec.txerr;
> > +           cf->data[7] = bec.rxerr;
> > +
> > +           break;
> > +   case C_CAN_ERROR_PASSIVE:
> > +           /* error passive state */
> > +           priv->can.can_stats.error_passive++;
> > +           priv->can.state = CAN_STATE_ERROR_PASSIVE;
> > +           cf->can_id |= CAN_ERR_CRTL;
> > +           if (rx_err_passive)
> > +                   cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> > +           if (bec.txerr > 127)
> > +                   cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> > +
> > +           cf->data[6] = bec.txerr;
> > +           cf->data[7] = bec.rxerr;
> > +           break;
> > +   case C_CAN_BUS_OFF:
> > +           /* bus-off state */
> > +           priv->can.state = CAN_STATE_BUS_OFF;
> > +           cf->can_id |= CAN_ERR_BUSOFF;
> > +           /*
> > +            * disable all interrupts in bus-off mode to ensure that
> > +            * the CPU is not hogged down
> > +            */
> > +           c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +           can_bus_off(dev);
> > +           break;
> > +   default:
> > +           break;
> > +   }
> > +
> > +   netif_receive_skb(skb);
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += cf->can_dlc;
> > +
> > +   return 1;
> > +}
> > +
> > +static int c_can_handle_bus_err(struct net_device *dev,
> > +                           enum c_can_lec_type lec_type)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct can_frame *cf;
> > +   struct sk_buff *skb;
> > +
> > +   /*
> > +    * early exit if no lec update or no error.
> > +    * no lec update means that no CAN bus event has been detected
> > +    * since CPU wrote 0x7 value to status reg.
> > +    */
> > +   if (lec_type == LEC_UNUSED || lec_type == LEC_NO_ERROR)
> > +           return 0;
> > +
> > +   /* propogate the error condition to the CAN stack */
> > +   skb = alloc_can_err_skb(dev, &cf);
> > +   if (unlikely(!skb))
> > +           return 0;
> > +
> > +   /*
> > +    * check for 'last error code' which tells us the
> > +    * type of the last error to occur on the CAN bus
> > +    */
> > +
> > +   /* common for all type of bus errors */
> > +   priv->can.can_stats.bus_error++;
> > +   stats->rx_errors++;
> > +   cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > +   cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > +
> > +   switch (lec_type) {
> > +   case LEC_STUFF_ERROR:
> > +           netdev_dbg(dev, "stuff error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_STUFF;
> > +           break;
> > +   case LEC_FORM_ERROR:
> > +           netdev_dbg(dev, "form error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_FORM;
> > +           break;
> > +   case LEC_ACK_ERROR:
> > +           netdev_dbg(dev, "ack error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> > +                           CAN_ERR_PROT_LOC_ACK_DEL);
> > +           break;
> > +   case LEC_BIT1_ERROR:
> > +           netdev_dbg(dev, "bit1 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT1;
> > +           break;
> > +   case LEC_BIT0_ERROR:
> > +           netdev_dbg(dev, "bit0 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT0;
> > +           break;
> > +   case LEC_CRC_ERROR:
> > +           netdev_dbg(dev, "CRC error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > +                           CAN_ERR_PROT_LOC_CRC_DEL);
> > +           break;
> > +   default:
> > +           break;
> > +   }
> > +
> > +   /* set a `lec` value so that we can check for updates later */
> > +   priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> > +
> > +   netif_receive_skb(skb);
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += cf->can_dlc;
> > +
> > +   return 1;
> > +}
> > +
> > +static int c_can_poll(struct napi_struct *napi, int quota) {
> > +   u16 irqstatus;
> > +   int lec_type = 0;
> > +   int work_done = 0;
> > +   struct net_device *dev = napi->dev;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> > +   if (!irqstatus)
> > +           goto end;
> > +
> > +   /* status events have the highest priority */
> > +   if (irqstatus == STATUS_INTERRUPT) {
> > +           priv->current_status = priv->read_reg(priv,
> > +                                   &priv->regs->status);
> > +
> > +           /* handle Tx/Rx events */
> > +           if (priv->current_status & STATUS_TXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_TXOK);
> > +
> > +           if (priv->current_status & STATUS_RXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_RXOK);
> > +
> > +           /* handle state changes */
> > +           if ((priv->current_status & STATUS_EWARN) &&
> > +                           (!(priv->last_status & STATUS_EWARN))) {
> > +                   netdev_dbg(dev, "entered error warning state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_ERROR_WARNING);
> > +           }
> > +           if ((priv->current_status & STATUS_EPASS) &&
> > +                           (!(priv->last_status & STATUS_EPASS))) {
> > +                   netdev_dbg(dev, "entered error passive state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_ERROR_PASSIVE);
> > +           }
> > +           if ((priv->current_status & STATUS_BOFF) &&
> > +                           (!(priv->last_status & STATUS_BOFF))) {
> > +                   netdev_dbg(dev, "entered bus off state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_BUS_OFF);
> > +           }
> > +
> > +           /* handle bus recovery events */
> > +           if ((!(priv->current_status & STATUS_BOFF)) &&
> > +                           (priv->last_status & STATUS_BOFF)) {
> > +                   netdev_dbg(dev, "left bus off state\n");
> > +                   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +           }
> > +           if ((!(priv->current_status & STATUS_EPASS)) &&
> > +                           (priv->last_status & STATUS_EPASS)) {
> > +                   netdev_dbg(dev, "left error passive state\n");
> > +                   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +           }
> > +
> > +           priv->last_status = priv->current_status;
> > +
> > +           /* handle lec errors on the bus */
> > +           lec_type = c_can_has_and_handle_berr(priv);
> > +           if (lec_type)
> > +                   work_done += c_can_handle_bus_err(dev, lec_type);
> > +   } else if ((irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
> > +                   (irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
> > +           /* handle events corresponding to receive message objects
> */
> > +           work_done += c_can_do_rx_poll(dev, (quota - work_done));
> > +   } else if ((irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
> > +                   (irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
> > +           /* handle events corresponding to transmit message objects
> */
> > +           c_can_do_tx(dev);
> > +   }
> > +
> > +end:
> > +   if (work_done < quota) {
> > +           napi_complete(napi);
> > +           /* enable all IRQs */
> > +           c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +   }
> > +
> > +   return work_done;
> > +}
> > +
> > +static irqreturn_t c_can_isr(int irq, void *dev_id) {
> > +   u16 irqstatus;
> > +   struct net_device *dev = (struct net_device *)dev_id;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> > +   if (!irqstatus)
> > +           return IRQ_NONE;
> > +
> > +   /* disable all interrupts and schedule the NAPI */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +   napi_schedule(&priv->napi);
> > +
> > +   return IRQ_HANDLED;
> > +}
> > +
> > +static int c_can_open(struct net_device *dev) {
> > +   int err;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* open the can device */
> > +   err = open_candev(dev);
> > +   if (err) {
> > +           netdev_err(dev, "failed to open can device\n");
> > +           return err;
> > +   }
> > +
> > +   /* register interrupt handler */
> > +   err = request_irq(dev->irq, &c_can_isr, IRQF_SHARED, dev->name,
> > +                           dev);
> > +   if (err < 0) {
> > +           netdev_err(dev, "failed to request interrupt\n");
> > +           goto exit_irq_fail;
> > +   }
> > +
> > +   /* start the c_can controller */
> > +   c_can_start(dev);
> > +
> > +   napi_enable(&priv->napi);
> > +   netif_start_queue(dev);
> > +
> > +   return 0;
> > +
> > +exit_irq_fail:
> > +   close_candev(dev);
> > +   return err;
> > +}
> > +
> > +static int c_can_close(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   netif_stop_queue(dev);
> > +   napi_disable(&priv->napi);
> > +   c_can_stop(dev);
> > +   free_irq(dev->irq, dev);
> > +   close_candev(dev);
> > +
> > +   return 0;
> > +}
> > +
> > +struct net_device *alloc_c_can_dev(void) {
> > +   struct net_device *dev;
> > +   struct c_can_priv *priv;
> > +
> > +   dev = alloc_candev(sizeof(struct c_can_priv),
> C_CAN_MSG_OBJ_TX_NUM);
> > +   if (!dev)
> > +           return NULL;
> > +
> > +   priv = netdev_priv(dev);
> > +   netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> > +
> > +   priv->dev = dev;
> > +   priv->can.bittiming_const = &c_can_bittiming_const;
> > +   priv->can.do_set_bittiming = c_can_set_bittiming;
>
> Please move the set_bittiming to the _open() or the _start() function.
> See b156fd0483c8f18b3cc544d9c400fe454458e16a. So we can get rid of the
> can.do_set_bittiming sooner or later.

Ok. Will make this change in V7.

> > +   priv->can.do_set_mode = c_can_set_mode;
> > +   priv->can.do_get_berr_counter = c_can_get_berr_counter;
> > +   priv->can.ctrlmode_supported = CAN_CTRLMODE_ONE_SHOT |
> > +                                   CAN_CTRLMODE_LOOPBACK |
> > +                                   CAN_CTRLMODE_LISTENONLY |
> > +                                   CAN_CTRLMODE_BERR_REPORTING;
> > +
> > +   return dev;
> > +}
> > +EXPORT_SYMBOL_GPL(alloc_c_can_dev);
> > +
> > +void free_c_can_dev(struct net_device *dev) {
> > +   free_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(free_c_can_dev);
> > +
> > +static const struct net_device_ops c_can_netdev_ops = {
> > +   .ndo_open = c_can_open,
> > +   .ndo_stop = c_can_close,
> > +   .ndo_start_xmit = c_can_start_xmit,
> > +};
> > +
> > +int register_c_can_dev(struct net_device *dev) {
> > +   dev->flags |= IFF_ECHO; /* we support local echo */
> > +   dev->netdev_ops = &c_can_netdev_ops;
> > +
> > +   return register_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(register_c_can_dev);
> > +
> > +void unregister_c_can_dev(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   unregister_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(unregister_c_can_dev);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("CAN bus driver for
> > +Bosch C_CAN controller");
> > diff --git a/drivers/net/can/c_can/c_can.h
> > b/drivers/net/can/c_can/c_can.h new file mode 100644 index
> > 0000000..bd094e6
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -0,0 +1,230 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef C_CAN_H
> > +#define C_CAN_H
> > +
> > +/* control register */
> > +#define CONTROL_TEST               BIT(7)
> > +#define CONTROL_CCE                BIT(6)
> > +#define CONTROL_DISABLE_AR BIT(5)
> > +#define CONTROL_ENABLE_AR  (0 << 5)
> > +#define CONTROL_EIE                BIT(3)
> > +#define CONTROL_SIE                BIT(2)
> > +#define CONTROL_IE         BIT(1)
> > +#define CONTROL_INIT               BIT(0)
> > +
> > +/* test register */
> > +#define TEST_RX                    BIT(7)
> > +#define TEST_TX1           BIT(6)
> > +#define TEST_TX2           BIT(5)
> > +#define TEST_LBACK         BIT(4)
> > +#define TEST_SILENT                BIT(3)
> > +#define TEST_BASIC         BIT(2)
> > +
> > +/* status register */
> > +#define STATUS_BOFF                BIT(7)
> > +#define STATUS_EWARN               BIT(6)
> > +#define STATUS_EPASS               BIT(5)
> > +#define STATUS_RXOK                BIT(4)
> > +#define STATUS_TXOK                BIT(3)
> > +
> > +/* error counter register */
> > +#define ERR_CNT_TEC_MASK   0xff
> > +#define ERR_CNT_TEC_SHIFT  0
> > +#define ERR_CNT_REC_SHIFT  8
> > +#define ERR_CNT_REC_MASK   (0x7f << ERR_CNT_REC_SHIFT)
> > +#define ERR_CNT_RP_SHIFT   15
> > +#define ERR_CNT_RP_MASK            (0x1 << ERR_CNT_RP_SHIFT)
> > +
> > +/* bit-timing register */
> > +#define BTR_BRP_MASK               0x3f
> > +#define BTR_BRP_SHIFT              0
> > +#define BTR_SJW_SHIFT              6
> > +#define BTR_SJW_MASK               (0x3 << BTR_SJW_SHIFT)
> > +#define BTR_TSEG1_SHIFT            8
> > +#define BTR_TSEG1_MASK             (0xf << BTR_TSEG1_SHIFT)
> > +#define BTR_TSEG2_SHIFT            12
> > +#define BTR_TSEG2_MASK             (0x7 << BTR_TSEG2_SHIFT)
> > +
> > +/* brp extension register */
> > +#define BRP_EXT_BRPE_MASK  0x0f
> > +#define BRP_EXT_BRPE_SHIFT 0
> > +
> > +/* IFx command request */
> > +#define IF_COMR_BUSY               BIT(15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR         BIT(7)
> > +#define IF_COMM_MASK               BIT(6)
> > +#define IF_COMM_ARB                BIT(5)
> > +#define IF_COMM_CONTROL            BIT(4)
> > +#define IF_COMM_CLR_INT_PND        BIT(3)
> > +#define IF_COMM_TXRQST             BIT(2)
> > +#define IF_COMM_DATAA              BIT(1)
> > +#define IF_COMM_DATAB              BIT(0)
> > +#define IF_COMM_ALL                (IF_COMM_MASK | IF_COMM_ARB | \
> > +                           IF_COMM_CONTROL | IF_COMM_TXRQST | \
> > +                           IF_COMM_DATAA | IF_COMM_DATAB)
> > +
> > +/* IFx arbitration */
> > +#define IF_ARB_MSGVAL              BIT(15)
> > +#define IF_ARB_MSGXTD              BIT(14)
> > +#define IF_ARB_TRANSMIT            BIT(13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT            BIT(15)
> > +#define IF_MCONT_MSGLST            BIT(14)
> > +#define IF_MCONT_CLR_MSGLST        (0 << 14)
> > +#define IF_MCONT_INTPND            BIT(13)
> > +#define IF_MCONT_UMASK             BIT(12)
> > +#define IF_MCONT_TXIE              BIT(11)
> > +#define IF_MCONT_RXIE              BIT(10)
> > +#define IF_MCONT_RMTEN             BIT(9)
> > +#define IF_MCONT_TXRQST            BIT(8)
> > +#define IF_MCONT_EOB               BIT(7)
> > +#define IF_MCONT_DLC_MASK  0xf
> > +
> > +/*
> > + * IFx register masks:
> > + * allow easy operation on 16-bit registers when the
> > + * argument is 32-bit instead
> > + */
> > +#define IFX_WRITE_LOW_16BIT(x)     ((x) & 0xFFFF)
> > +#define IFX_WRITE_HIGH_16BIT(x)    (((x) & 0xFFFF0000) >> 16)
> > +
> > +/* message object split */
> > +#define C_CAN_NO_OF_OBJECTS        32
> > +#define C_CAN_MSG_OBJ_RX_NUM       16
> > +#define C_CAN_MSG_OBJ_TX_NUM       16
> > +
> > +#define C_CAN_MSG_OBJ_RX_FIRST     1
> > +#define C_CAN_MSG_OBJ_RX_LAST      (C_CAN_MSG_OBJ_RX_FIRST + \
> > +                           C_CAN_MSG_OBJ_RX_NUM - 1)
> > +
> > +#define C_CAN_MSG_OBJ_TX_FIRST     (C_CAN_MSG_OBJ_RX_LAST + 1)
> > +#define C_CAN_MSG_OBJ_TX_LAST      (C_CAN_MSG_OBJ_TX_FIRST + \
> > +                           C_CAN_MSG_OBJ_TX_NUM - 1)
> > +
> > +#define C_CAN_MSG_OBJ_RX_SPLIT     9
> > +#define C_CAN_MSG_RX_LOW_LAST      (C_CAN_MSG_OBJ_RX_SPLIT - 1)
> > +
> > +#define C_CAN_NEXT_MSG_OBJ_MASK    (C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS        0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT   0x8000
> > +
> > +/* global interrupt masks */
> > +#define ENABLE_ALL_INTERRUPTS      1
> > +#define DISABLE_ALL_INTERRUPTS     0
> > +
> > +/* minimum timeout for checking BUSY status */
> > +#define MIN_TIMEOUT_VALUE  6
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT  C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > +   u16 com_req;
> > +   u16 com_mask;
> > +   u16 mask1;
> > +   u16 mask2;
> > +   u16 arb1;
> > +   u16 arb2;
> > +   u16 msg_cntrl;
> > +   u16 data[4];
> > +   u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > +   u16 control;
> > +   u16 status;
> > +   u16 err_cnt;
> > +   u16 btr;
> > +   u16 interrupt;
> > +   u16 test;
> > +   u16 brp_ext;
> > +   u16 _reserved1;
> > +   struct c_can_if_regs ifregs[2]; /* [0] = IF1 and [1] = IF2 */
> > +   u16 _reserved2[8];
> > +   u16 txrqst1;
> > +   u16 txrqst2;
> > +   u16 _reserved3[6];
> > +   u16 newdat1;
> > +   u16 newdat2;
> > +   u16 _reserved4[6];
> > +   u16 intpnd1;
> > +   u16 intpnd2;
> > +   u16 _reserved5[6];
> > +   u16 msgval1;
> > +   u16 msgval2;
> > +   u16 _reserved6[6];
> > +};
> > +
> > +/* c_can lec values */
> > +enum c_can_lec_type {
> > +   LEC_NO_ERROR = 0,
> > +   LEC_STUFF_ERROR,
> > +   LEC_FORM_ERROR,
> > +   LEC_ACK_ERROR,
> > +   LEC_BIT1_ERROR,
> > +   LEC_BIT0_ERROR,
> > +   LEC_CRC_ERROR,
> > +   LEC_UNUSED,
> > +};
> > +
> > +/*
> > + * c_can error types:
> > + * Bus errors (BUS_OFF, ERROR_WARNING, ERROR_PASSIVE) are supported
> > +*/ enum c_can_bus_error_types {
> > +   C_CAN_NO_ERROR = 0,
> > +   C_CAN_BUS_OFF,
> > +   C_CAN_ERROR_WARNING,
> > +   C_CAN_ERROR_PASSIVE,
> > +};
>
> nitpick: are the defines, enums and structs needed in more than one c
> file? If not, please move them into the c-file where they are used.

Well most of the strcuts/defines are useful in both `c_can.c` and
`c_can_platform.c`, but I will explore how to separate the rest in
the respective c-files. But inititally we agreed to a *sja1000* like
approach and hence this placement in h-file.

> > +
> > +/* c_can private data structure */
> > +struct c_can_priv {
> > +   struct can_priv can;    /* must be the first member */
> > +   struct napi_struct napi;
> > +   struct net_device *dev;
> > +   int tx_object;
> > +   int current_status;
> > +   int last_status;
> > +   u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> > +   void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> > +   struct c_can_regs __iomem *regs;
> > +   unsigned long irq_flags; /* for request_irq() */
> > +   unsigned int tx_next;
> > +   unsigned int tx_echo;
> > +   void *priv;             /* for board-specific data */
> > +};
> > +
> > +struct net_device *alloc_c_can_dev(void); void free_c_can_dev(struct
> > +net_device *dev); int register_c_can_dev(struct net_device *dev);
> > +void unregister_c_can_dev(struct net_device *dev);
> > +
> > +#endif /* C_CAN_H */
> > diff --git a/drivers/net/can/c_can/c_can_platform.c
> > b/drivers/net/can/c_can/c_can_platform.c
> > new file mode 100644
> > index 0000000..0fc314e
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -0,0 +1,207 @@
> > +/*
> > + * Platform CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/if_arp.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/list.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +
> > +#include <linux/can/dev.h>
> > +
> > +#include "c_can.h"
> > +
> > +/*
> > + * 16-bit c_can registers can be arranged differently in the memory
> > + * architecture of different implementations. For example: 16-bit
> > + * registers can be aligned to a 16-bit boundary or 32-bit boundary
> etc.
> > + * Handle the same by providing a common read/write interface.
> > + */
> > +static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv
> *priv,
> > +                                           void *reg)
> > +{
> > +   return readw(reg);
> > +}
> > +
> > +static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv
> *priv,
> > +                                           void *reg, u16 val)
> > +{
> > +   writew(val, reg);
> > +}
> > +
> > +static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > +                                           void *reg)
> > +{
> > +   return readw(reg + (long)reg - (long)priv->regs); }
> > +
> > +static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > +                                           void *reg, u16 val)
> > +{
> > +   writew(val, reg + (long)reg - (long)priv->regs); }
> > +
> > +static int __devinit c_can_plat_probe(struct platform_device *pdev)
> {
> > +   int ret;
> > +   void __iomem *addr;
> > +   struct net_device *dev;
> > +   struct c_can_priv *priv;
> > +   struct resource *mem, *irq;
> > +   struct clk *clk;
> > +
> > +   /* get the appropriate clk */
> > +   clk = clk_get(&pdev->dev, NULL);
> > +   if (IS_ERR(clk)) {
> > +           dev_err(&pdev->dev, "no clock defined\n");
> > +           ret = -ENODEV;
> > +           goto exit;
> > +   }
> > +
> > +   /* get the platform data */
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +   if (!mem || (irq <= 0)) {
> > +           ret = -ENODEV;
> > +           goto exit_free_clk;
> > +   }
> > +
> > +   if (!request_mem_region(mem->start, resource_size(mem),
> > +                           KBUILD_MODNAME)) {
> > +           dev_err(&pdev->dev, "resource unavailable\n");
> > +           ret = -ENODEV;
> > +           goto exit_free_clk;
> > +   }
> > +
> > +   addr = ioremap(mem->start, resource_size(mem));
> > +   if (!addr) {
> > +           dev_err(&pdev->dev, "failed to map can port\n");
> > +           ret = -ENOMEM;
> > +           goto exit_release_mem;
> > +   }
> > +
> > +   /* allocate the c_can device */
> > +   dev = alloc_c_can_dev();
> > +   if (!dev) {
> > +           ret = -ENOMEM;
> > +           goto exit_iounmap;
> > +   }
> > +
> > +   priv = netdev_priv(dev);
> > +
> > +   dev->irq = irq->start;
> > +   priv->regs = addr;
> > +   priv->can.clock.freq = clk_get_rate(clk);
> > +   priv->priv = clk;
> > +
> > +   switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> > +   case IORESOURCE_MEM_32BIT:
> > +           priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> > +           priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> > +           break;
> > +   case IORESOURCE_MEM_16BIT:
> > +   default:
> > +           priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > +           priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > +           break;
> > +   }
> > +
> > +   platform_set_drvdata(pdev, dev);
> > +   SET_NETDEV_DEV(dev, &pdev->dev);
> > +
> > +   ret = register_c_can_dev(dev);
> > +   if (ret) {
> > +           dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> > +                   KBUILD_MODNAME, ret);
> > +           goto exit_free_device;
> > +   }
> > +
> > +   dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> > +            KBUILD_MODNAME, priv->regs, dev->irq);
> > +   return 0;
> > +
> > +exit_free_device:
> > +   platform_set_drvdata(pdev, NULL);
> > +   free_c_can_dev(dev);
> > +exit_iounmap:
> > +   iounmap(addr);
> > +exit_release_mem:
> > +   release_mem_region(mem->start, resource_size(mem));
> > +exit_free_clk:
> > +   clk_put(clk);
> > +exit:
> > +   dev_err(&pdev->dev, "probe failed\n");
> > +
> > +   return ret;
> > +}
> > +
> > +static int __devexit c_can_plat_remove(struct platform_device *pdev)
> > +{
> > +   struct net_device *dev = platform_get_drvdata(pdev);
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct resource *mem;
> > +
> > +   unregister_c_can_dev(dev);
> > +   platform_set_drvdata(pdev, NULL);
> > +
> > +   free_c_can_dev(dev);
> > +   iounmap(priv->regs);
> > +
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   release_mem_region(mem->start, resource_size(mem));
> > +
> > +   clk_put(priv->priv);
> > +
> > +   return 0;
> > +}
> > +
> > +static struct platform_driver c_can_plat_driver = {
> > +   .driver = {
> > +           .name = KBUILD_MODNAME,
> > +           .owner = THIS_MODULE,
> > +   },
> > +   .probe = c_can_plat_probe,
> > +   .remove = __devexit_p(c_can_plat_remove), };
> > +
> > +static int __init c_can_plat_init(void) {
> > +   return platform_driver_register(&c_can_plat_driver);
> > +}
> > +module_init(c_can_plat_init);
> > +
> > +static void __exit c_can_plat_exit(void) {
> > +   platform_driver_unregister(&c_can_plat_driver);
> > +}
> > +module_exit(c_can_plat_exit);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("Platform CAN bus
> driver
> > +for Bosch C_CAN controller");
>
> regards, Marc

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Marc Kleine-Budde @ 2011-02-09 10:46 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Bhupesh Sharma
In-Reply-To: <4D526F5A.6030706-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 935 bytes --]

On 02/09/2011 11:41 AM, Wolfgang Grandegger wrote:
> Hi Marc,
> 
> On 02/09/2011 10:27 AM, Marc Kleine-Budde wrote:
>> On 02/09/2011 06:03 AM, Bhupesh Sharma wrote:
>>> +	if (flags & IF_ARB_TRANSMIT)
>>> +		frame->can_id |= CAN_RTR_FLAG;
>>
>> Can you please only copy the data to the frame if the rtr flag isn't
>> set. Not all driver do this yet, but we've agreed to do so.
> 
> Yes, I remember the patch
> 
> [PATCH v2 0/9] can: don't copy data to rx'ed RTR frames
> 
> What happend with it?

*g* - I didn't compile tested all drivers, and David found an error. I'm
busy right now but I've requested a timeslot for it.

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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2011-02-09 10:41 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Bhupesh Sharma, socketcan-core, netdev
In-Reply-To: <4D525DF8.6050409@pengutronix.de>

Hi Marc,

On 02/09/2011 10:27 AM, Marc Kleine-Budde wrote:
> On 02/09/2011 06:03 AM, Bhupesh Sharma wrote:
>> +	if (flags & IF_ARB_TRANSMIT)
>> +		frame->can_id |= CAN_RTR_FLAG;
> 
> Can you please only copy the data to the frame if the rtr flag isn't
> set. Not all driver do this yet, but we've agreed to do so.

Yes, I remember the patch

[PATCH v2 0/9] can: don't copy data to rx'ed RTR frames

What happend with it?

Wolfgang.

^ permalink raw reply

* MSG_DONTROUTE with IP_PKTINFO(ipi_ifindex)
From: Joakim Tjernlund @ 2011-02-09 10:36 UTC (permalink / raw)
  To: netdev


How does sendmsg(2) work w.r.t IP_PKTINFO(ipi_ifindex) != 0
and MSG_DONTROUTE set?

man 7 ip has this to say about ipi_ifindex:

       IP_PKTINFO (since Linux 2.2)
              Pass  an  IP_PKTINFO  ancillary  message that contains a pktinfo
              structure that supplies  some  information  about  the  incoming
              packet.   This  only  works  for datagram oriented sockets.  The
              argument is a flag that tells the socket whether the  IP_PKTINFO
              message should be passed or not.  The message itself can only be
              sent/retrieved as control message with a packet using recvmsg(2)
              or sendmsg(2).

                  struct in_pktinfo {
                      unsigned int   ipi_ifindex;  /* Interface index */
                      struct in_addr ipi_spec_dst; /* Local address */
                      struct in_addr ipi_addr;     /* Header Destination
                                                      address */
                  };

              ipi_ifindex  is the unique index of the interface the packet was
              received on.  ipi_spec_dst is the local address  of  the  packet
              and  ipi_addr  is  the destination address in the packet header.
              If IP_PKTINFO is passed to sendmsg(2) and  ipi_spec_dst  is  not
              zero,  then it is used as the local source address for the rout‐
              ing table lookup and for setting up  IP  source  route  options.
              When  ipi_ifindex  is not zero, the primary local address of the
              interface specified by the index overwrites ipi_spec_dst for the
              routing table lookup.

So if ipi_ifindex is set with MSG_DONTROUTE, will linux use the I/F in ipi_index
or will it still lookup the dst address and use that instead?

I am asking as I got multiple ppp I/F between the same 2 routers which have
the same local and remote address(unnumbered ppp interfaces) and I want
to specify the interface to use as src and dst addr aren't unique.

Also, suppose I only have ipi_spec_dst and ipi_addr set and multiple interfaces match,
will the packet be sent on one of the matching interfaces or on all matching interfaces?

 Jocke


^ permalink raw reply

* (unknown), 
From:  JMC Service® @ 2011-02-09 11:14 UTC (permalink / raw)


We Loan at 3%, Interested person(s) from any part of the world, should contact us via  the email below for more information.

Names........
Amount Needed.....
Phone Number....... 

Regards,
Loan Solution.
loansolution@onfruit.com




^ permalink raw reply

* RE: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2011-02-09 10:31 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wolfgang Grandegger
In-Reply-To: <4D525DF8.6050409-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Hi Marc,

Thanks for your review comments.
Please see my comments inline:

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> Sent: Wednesday, February 09, 2011 2:57 PM
> To: Bhupesh SHARMA
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH net-next-2.6 v6 1/1] can: c_can: Added support for
> Bosch C_CAN controller
>
> On 02/09/2011 06:03 AM, Bhupesh Sharma wrote:
> > Bosch C_CAN controller is a full-CAN implementation which is
> compliant
> > to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can
> > be obtained from:
> > http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
> > c_can/users_manual_c_can.pdf
> >
> > This patch adds the support for this controller.
> > The following are the design choices made while writing the
> controller
> > driver:
> > 1. Interface Register set IF1 has be used only in the current design.
> > 2. Out of the 32 Message objects available, 16 are kept aside for RX
> >    purposes and the rest for TX purposes.
> > 3. NAPI implementation is such that both the TX and RX paths function
> >    in polling mode.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>
> The driver looks quite good, some comments inline, most of them
> nitpicking and or style related.
>
> Have a look at the netif_stop_queue(). In the at91 driver there are two
> possibilities that to stop the queue. First the next tx mailbox is
> still in use, second we have a wrap around. But your hardware is a bit
> different. Anyways a second look doesn't harm.

As the Tx/Rx path of my driver are based on at91 driver, I have earlier
gone through the possibility of stopping the tx queue in the two cases as
you mentioned above :)

As per at91 specs, MRDY=0 signifies:
"Mailbox data registers cannot be read/written by the software application"

But, after reading the Bosch C_CAN specs Transmission Request Register(TxRqst)'s
bits if set to 1, signify that the transmission of this Message Object is
requested and is not yet done. If you agree we can add a check against the same here.
Please do go through the Bosch C_CAN specs for details:
http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
c_can/users_manual_c_can.pdf

> > ---
> > Changes since V5:
> > 1. Seperated the state change and bus error handling paths.
> > 2. Added logic to write LEC value to 0x7 from CPU to check for
> updates
> >    later.
> > 3. Corrected the ERROR_WARNING handling logic to correctly send error
> >    frames on the bus.
> >
> >  drivers/net/can/Kconfig                |    2 +
> >  drivers/net/can/Makefile               |    1 +
> >  drivers/net/can/c_can/Kconfig          |   15 +
> >  drivers/net/can/c_can/Makefile         |    8 +
> >  drivers/net/can/c_can/c_can.c          |  993
> ++++++++++++++++++++++++++++++++
> >  drivers/net/can/c_can/c_can.h          |  230 ++++++++
> >  drivers/net/can/c_can/c_can_platform.c |  207 +++++++
> >  7 files changed, 1456 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/net/can/c_can/Kconfig  create mode 100644
> > drivers/net/can/c_can/Makefile  create mode 100644
> > drivers/net/can/c_can/c_can.c  create mode 100644
> > drivers/net/can/c_can/c_can.h  create mode 100644
> > drivers/net/can/c_can/c_can_platform.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > 5dec456..1d699e3 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -115,6 +115,8 @@ source "drivers/net/can/mscan/Kconfig"
> >
> >  source "drivers/net/can/sja1000/Kconfig"
> >
> > +source "drivers/net/can/c_can/Kconfig"
> > +
> >  source "drivers/net/can/usb/Kconfig"
> >
> >  source "drivers/net/can/softing/Kconfig"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index
> > 53c82a7..24ebfe8 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -13,6 +13,7 @@ obj-y                             += softing/
> >
> >  obj-$(CONFIG_CAN_SJA1000)  += sja1000/
> >  obj-$(CONFIG_CAN_MSCAN)            += mscan/
> > +obj-$(CONFIG_CAN_C_CAN)            += c_can/
> >  obj-$(CONFIG_CAN_AT91)             += at91_can.o
> >  obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
> >  obj-$(CONFIG_CAN_MCP251X)  += mcp251x.o
> > diff --git a/drivers/net/can/c_can/Kconfig
> > b/drivers/net/can/c_can/Kconfig new file mode 100644 index
> > 0000000..ffb9773
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Kconfig
> > @@ -0,0 +1,15 @@
> > +menuconfig CAN_C_CAN
> > +   tristate "Bosch C_CAN devices"
> > +   depends on CAN_DEV && HAS_IOMEM
> > +
> > +if CAN_C_CAN
> > +
> > +config CAN_C_CAN_PLATFORM
> > +   tristate "Generic Platform Bus based C_CAN driver"
> > +   ---help---
> > +     This driver adds support for the C_CAN chips connected to
> > +     the "platform bus" (Linux abstraction for directly to the
> > +     processor attached devices) which can be found on various
> > +     boards from ST Microelectronics (http://www.st.com)
> > +     like the SPEAr1310 and SPEAr320 evaluation boards.
> > +endif
> > diff --git a/drivers/net/can/c_can/Makefile
> > b/drivers/net/can/c_can/Makefile new file mode 100644 index
> > 0000000..9273f6d
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Makefile
> > @@ -0,0 +1,8 @@
> > +#
> > +#  Makefile for the Bosch C_CAN controller drivers.
> > +#
> > +
> > +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> > +obj-$(CONFIG_CAN_C_CAN_PLATFORM) += c_can_platform.o
> > +
> > +ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> > diff --git a/drivers/net/can/c_can/c_can.c
> > b/drivers/net/can/c_can/c_can.c new file mode 100644 index
> > 0000000..7ef4aa9
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -0,0 +1,993 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * TX and RX NAPI implementation has been borrowed from at91 CAN
> > +driver
> > + * written by:
> > + * Copyright
> > + * (C) 2007 by Hans J. Koch <hjk-vqZO0P4V72/QD6PfKP4TzA@public.gmane.org>
> > + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/if_arp.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/list.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +
> > +#include <linux/can.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +
> > +#include "c_can.h"
> > +
> > +static struct can_bittiming_const c_can_bittiming_const = {
> > +   .name = KBUILD_MODNAME,
> > +   .tseg1_min = 2,         /* Time segment 1 = prop_seg + phase_seg1
> */
> > +   .tseg1_max = 16,
> > +   .tseg2_min = 1,         /* Time segment 2 = phase_seg2 */
> > +   .tseg2_max = 8,
> > +   .sjw_max = 4,
> > +   .brp_min = 1,
> > +   .brp_max = 1024,        /* 6-bit BRP field + 4-bit BRPE field*/
> > +   .brp_inc = 1,
> > +};
> > +
> > +static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg) {
> > +   u32 val = priv->read_reg(priv, reg);
> > +   val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> > +   return val;
> > +}
> > +
> > +static void c_can_enable_all_interrupts(struct c_can_priv *priv,
> > +                                           int enable)
> > +{
> > +   unsigned int cntrl_save = priv->read_reg(priv,
> > +                                           &priv->regs->control);
> > +
> > +   if (enable)
> > +           cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> > +   else
> > +           cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> > +
> > +   priv->write_reg(priv, &priv->regs->control, cntrl_save); }
> > +
> > +static inline int c_can_check_busy_status(struct c_can_priv *priv,
> > +int iface) {
> > +   int count = MIN_TIMEOUT_VALUE;
> > +
> > +   while (count && priv->read_reg(priv,
> > +                           &priv->regs->ifregs[iface].com_req) &
> > +                           IF_COMR_BUSY) {
> > +           count--;
> > +           udelay(1);
> > +   }
> > +
> > +   return count;
>
> it's an unusual return value...maybe return 0 on success and -EBUSY
> otherwise?

Hmm.. this will add the checking MIN_TIMEOUT_VALUE against 0 here,
instead of "c_can_object_get" and "c_can_object_put" routines.
If you persist we can add the same in V7 though.. :)

> > +}
> > +
> > +static inline void c_can_object_get(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +                   IFX_WRITE_LOW_16BIT(objno));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           netdev_err(dev, "timed out in object get\n"); }
> > +
> > +static inline void c_can_object_put(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +                   (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +                   IFX_WRITE_LOW_16BIT(objno));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           netdev_err(dev, "timed out in object put\n"); }
> > +
> > +static void c_can_write_msg_object(struct net_device *dev,
> > +                   int iface, struct can_frame *frame, int objno) {
> > +   int i;
> > +   u16 flags = 0;
> > +   unsigned int id;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (!(frame->can_id & CAN_RTR_FLAG))
> > +           flags |= IF_ARB_TRANSMIT;
> > +
> > +   if (frame->can_id & CAN_EFF_FLAG) {
> > +           id = frame->can_id & CAN_EFF_MASK;
> > +           flags |= IF_ARB_MSGXTD;
> > +   } else
> > +           id = ((frame->can_id & CAN_SFF_MASK) << 18);
> > +
> > +   flags |= IF_ARB_MSGVAL;
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +                           IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, flags |
> > +                           IFX_WRITE_HIGH_16BIT(id));
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           priv->write_reg(priv, &priv->regs->ifregs[iface].data[i /
> 2],
> > +                           frame->data[i] | (frame->data[i + 1] << 8));
> > +   }
> > +
> > +   /* enable interrupt for this message object */
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> > +                   (frame->can_dlc & 0xf));
>                                         ^^^^^
>
> the masking should not be needed, as you're using
> can_dropped_invalid_skb()

Ok.

> > +   c_can_object_put(dev, iface, objno, IF_COMM_ALL); }
> > +
> > +static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> > +
> > +}
> > +
> > +static inline void c_can_activate_all_lower_rx_msg_obj(struct
> net_device *dev,
> > +                                           int iface,
> > +                                           int ctrl_mask)
> > +{
> > +   int i;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_MSG_RX_LOW_LAST; i++)
> {
> > +           priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                           ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                                   IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +           c_can_object_put(dev, iface, i, IF_COMM_CONTROL);
> > +   }
> > +}
> > +
> > +static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                           IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL); }
> > +
> > +static void c_can_handle_lost_msg_obj(struct net_device *dev,
> > +                                   int iface, int objno)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   netdev_err(dev, "msg lost in buffer %d\n", objno);
> > +
> > +   c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> ~IF_COMM_TXRQST);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +                   IF_MCONT_CLR_MSGLST);
> > +
> > +   c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
> > +
> > +   /* create an error msg */
> > +   skb = alloc_can_err_skb(dev, &frame);
> > +   if (unlikely(!skb))
> > +           return;
> > +
> > +   frame->can_id |= CAN_ERR_CRTL;
> > +   frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> > +   stats->rx_errors++;
> > +   stats->rx_over_errors++;
> > +
> > +   netif_receive_skb(skb);
> > +}
> > +
> > +static int c_can_read_msg_object(struct net_device *dev, int iface,
> > +int ctrl) {
> > +   u16 flags, data;
> > +   int i;
> > +   unsigned int val;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   skb = alloc_can_skb(dev, &frame);
> > +   if (!skb) {
> > +           stats->rx_dropped++;
> > +           return -ENOMEM;
> > +   }
> > +
> > +   frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           data = priv->read_reg(priv,
> > +                           &priv->regs->ifregs[iface].data[i / 2]);
> > +           frame->data[i] = data;
> > +           frame->data[i + 1] = data >> 8;
> > +   }
> > +
> > +   flags = priv->read_reg(priv, &priv->regs-
> >ifregs[iface].arb2);
> > +   val = priv->read_reg(priv, &priv->regs->ifregs[iface].arb1) |
> > +           (flags << 16);
> > +
> > +   if (flags & IF_ARB_MSGXTD)
> > +           frame->can_id = (val & CAN_EFF_MASK) | CAN_EFF_FLAG;
> > +   else
> > +           frame->can_id = (val >> 18) & CAN_SFF_MASK;
> > +
> > +   if (flags & IF_ARB_TRANSMIT)
> > +           frame->can_id |= CAN_RTR_FLAG;
>
> Can you please only copy the data to the frame if the rtr flag isn't
> set. Not all driver do this yet, but we've agreed to do so.

Ok.

> > +
> > +   netif_receive_skb(skb);
> > +
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += frame->can_dlc;
> > +
> > +   return 0;
> > +}
> > +
> > +static void c_can_setup_receive_object(struct net_device *dev, int
> iface,
> > +                                   int objno, unsigned int mask,
> > +                                   unsigned int id, unsigned int mcont) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].mask1,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].mask2,
> > +                   IFX_WRITE_HIGH_16BIT(mask));
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +                   IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2,
> > +                   (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> mcont);
> > +   c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> ~IF_COMM_TXRQST);
> > +
> > +   netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1)); }
> > +
> > +static void c_can_inval_msg_object(struct net_device *dev, int
> iface,
> > +int objno) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb1, 0);
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, 0);
> > +   priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, 0);
> > +
> > +   c_can_object_put(dev, iface, objno, IF_COMM_ARB |
> IF_COMM_CONTROL);
> > +
> > +   netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1)); }
> > +
> > +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> > +                                   struct net_device *dev)
> > +{
> > +   u32 msg_obj_no;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct can_frame *frame = (struct can_frame *)skb->data;
> > +
> > +   if (can_dropped_invalid_skb(dev, skb))
> > +           return NETDEV_TX_OK;
> > +
> > +   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);
> > +   can_put_echo_skb(skb, dev, msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> > +
> > +   priv->tx_next++;
> > +   if ((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) == 0)
> > +           netif_stop_queue(dev);
>
> It it possible, that the "tx_next" object isn't available, yet?
> Understanding your code right. It's not possible, because you have a
> wrap around (which is handled above) first.

Please see explanation above.

> > +
> > +   return NETDEV_TX_OK;
> > +}
> > +
> > +static int c_can_set_bittiming(struct net_device *dev) {
> > +   unsigned int reg_btr, reg_brpe, ctrl_save;
> > +   u8 brp, brpe, sjw, tseg1, tseg2;
> > +   u32 ten_bit_brp;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   const struct can_bittiming *bt = &priv->can.bittiming;
> > +
> > +   /* c_can provides a 6-bit brp and 4-bit brpe fields */
> > +   ten_bit_brp = bt->brp - 1;
> > +   brp = ten_bit_brp & BTR_BRP_MASK;
> > +   brpe = ten_bit_brp >> 6;
> > +
> > +   sjw = bt->sjw - 1;
> > +   tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> > +   tseg2 = bt->phase_seg2 - 1;
> > +   reg_btr = brp | (sjw << BTR_SJW_SHIFT) | (tseg1 <<
> BTR_TSEG1_SHIFT) |
> > +                   (tseg2 << BTR_TSEG2_SHIFT);
> > +   reg_brpe = brpe & BRP_EXT_BRPE_MASK;
> > +
> > +   netdev_info(dev,
> > +           "setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
> > +
> > +   ctrl_save = priv->read_reg(priv, &priv->regs->control);
> > +   priv->write_reg(priv, &priv->regs->control,
> > +                   ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > +   priv->write_reg(priv, &priv->regs->btr, reg_btr);
> > +   priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
> > +   priv->write_reg(priv, &priv->regs->control, ctrl_save);
> > +
> > +   return 0;
> > +}
> > +
> > +/*
> > + * Configure C_CAN message objects for Tx and Rx purposes:
> > + * C_CAN provides a total of 32 message objects that can be
> > +configured
> > + * either for Tx or Rx purposes. Here the first 16 message objects
> > +are used as
> > + * a reception FIFO. The end of reception FIFO is signified by the
> > +EoB bit
> > + * being SET. The remaining 16 message objects are kept aside for Tx
> purposes.
> > + * See user guide document for further details on configuring
> message
> > + * objects.
> > + */
> > +static void c_can_configure_msg_objects(struct net_device *dev) {
> > +   int i;
> > +
> > +   /* first invalidate all message objects */
> > +   for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_NO_OF_OBJECTS; i++)
> > +           c_can_inval_msg_object(dev, 0, i);
> > +
> > +   /* 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); }
> > +
> > +/*
> > + * Configure C_CAN chip:
> > + * - enable/disable auto-retransmission
> > + * - set operating mode
> > + * - configure message objects
> > + */
> > +static void c_can_chip_config(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> > +           /* disable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_DISABLE_AR);
> > +   else
> > +           /* enable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_ENABLE_AR);
> > +
> > +   if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> > +                                   CAN_CTRLMODE_LOOPBACK)) {
> > +           /* loopback + silent mode : useful for hot self-test */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test,
> > +                           TEST_LBACK | TEST_SILENT);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > +           /* loopback mode : useful for self-test function */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> > +           /* silent mode : bus-monitoring mode */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
> > +   } else
> > +           /* normal mode*/
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> > +
> > +   /* configure message objects */
> > +   c_can_configure_msg_objects(dev);
> > +
> > +   /* set a `lec` value so that we can check for updates later */
> > +   priv->write_reg(priv, &priv->regs->status, LEC_UNUSED); }
> > +
> > +static void c_can_start(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* enable status change, error and module interrupts */
> > +   c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +
> > +   /* basic c_can configuration */
> > +   c_can_chip_config(dev);
> > +
> > +   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > +   /* reset tx helper pointers */
> > +   priv->tx_next = priv->tx_echo = 0;
> > +}
> > +
> > +static void c_can_stop(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   /* set the state as STOPPED */
> > +   priv->can.state = CAN_STATE_STOPPED; }
> > +
> > +static int c_can_set_mode(struct net_device *dev, enum can_mode
> mode)
> > +{
> > +   switch (mode) {
> > +   case CAN_MODE_START:
> > +           c_can_start(dev);
> > +           netif_wake_queue(dev);
> > +           break;
> > +   default:
> > +           return -EOPNOTSUPP;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int c_can_get_berr_counter(const struct net_device *dev,
> > +                                   struct can_berr_counter *bec)
> > +{
> > +   unsigned int reg_err_counter;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +   bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
> > +                           ERR_CNT_REC_SHIFT;
> > +   bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
> > +
> > +   return 0;
> > +}
> > +
> > +/*
> > + * theory of operation:
> > + *
> > + * priv->tx_echo holds the number of the oldest can_frame put for
> > + * transmission into the hardware, but not yet ACKed by the CAN tx
> > + * complete IRQ.
> > + *
> > + * We iterate from priv->tx_echo to priv->tx_next and check if the
> > + * packet has been transmitted, echo it back to the CAN framework.
> > + * If we discover a not yet transmitted package, stop looking for
> more.
> > + */
> > +static void c_can_do_tx(struct net_device *dev) {
> > +   u32 val;
> > +   u32 msg_obj_no;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +
> > +   for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv-
> >tx_echo++) {
> > +           msg_obj_no = get_tx_echo_msg_obj(priv);
> > +           c_can_inval_msg_object(dev, 0, msg_obj_no);
> > +           val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> > +           if (!(val & (1 << msg_obj_no))) {
> > +                   can_get_echo_skb(dev,
> > +                                   msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> > +                   stats->tx_bytes += priv->read_reg(priv,
> > +                                   &priv->regs->ifregs[0].msg_cntrl)
> > +                                   & IF_MCONT_DLC_MASK;
> > +                   stats->tx_packets++;
> > +           }
> > +   }
> > +
> > +   /* restart queue if wrap-up or if queue stalled on last pkt */
> > +   if (((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) != 0) ||
> > +                   ((priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) == 0))
> > +           netif_wake_queue(dev);
> > +}
> > +
> > +/*
> > + * theory of operation:
> > + *
> > + * c_can core saves a received CAN message into the first free
> > +message
> > + * object it finds free (starting with the lowest). Bits NEWDAT and
> > + * INTPND are set for this message object indicating that a new
> > +message
> > + * has arrived. To work-around this issue, we keep two groups of
> > +message
> > + * objects whose partitioning is defined by C_CAN_MSG_OBJ_RX_SPLIT.
> > + *
> > + * To ensure in-order frame reception we use the following
> > + * approach while re-activating a message object to receive further
> > + * frames:
> > + * - if the current message object number is lower than
> > + *   C_CAN_MSG_RX_LOW_LAST, do not clear the NEWDAT bit while
> clearing
> > + *   the INTPND bit.
> > + * - if the current message object number is equal to
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of all lower
> > + *   receive message objects.
> > + * - if the current message object number is greater than
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of
> > + *   only this message object.
> > + */
> > +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, &priv->regs->intpnd1);
> > +
> > +   for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> > +                   msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
> > +                   val = c_can_read_reg32(priv, &priv->regs->intpnd1),
> > +                   msg_obj++) {
> > +           /*
> > +            * as interrupt pending register's bit n-1 corresponds to
> > +            * message object n, we need to handle the same properly.
> > +            */
> > +           if (val & (1 << (msg_obj - 1))) {
> > +                   c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
> > +                                   ~IF_COMM_TXRQST);
> > +                   msg_ctrl_save = priv->read_reg(priv,
> > +                                   &priv->regs->ifregs[0].msg_cntrl);
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_EOB)
> > +                           return num_rx_pkts;
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_MSGLST) {
> > +                           c_can_handle_lost_msg_obj(dev, 0, msg_obj);
> > +                           num_rx_pkts++;
> > +                           quota--;
> > +                           continue;
> > +                   }
> > +
> > +                   if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
> > +                           continue;
> > +
> > +                   /* read the data from the message object */
> > +                   c_can_read_msg_object(dev, 0, msg_ctrl_save);
> > +
> > +                   if (msg_obj < C_CAN_MSG_RX_LOW_LAST)
> > +                           c_can_mark_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj > C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate this msg obj */
> > +                           c_can_activate_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj == C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate all lower message objects */
> > +                           c_can_activate_all_lower_rx_msg_obj(dev,
> > +                                           0, msg_ctrl_save);
> > +
> > +                   num_rx_pkts++;
> > +                   quota--;
> > +           }
> > +   }
> > +
> > +   return num_rx_pkts;
> > +}
> > +
> > +static inline int c_can_has_and_handle_berr(struct c_can_priv *priv)
> > +{
> > +   return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> > +           (priv->current_status & LEC_UNUSED); }
> > +
> > +static int c_can_handle_state_change(struct net_device *dev,
> > +                           enum c_can_bus_error_types error_type) {
> > +   unsigned int reg_err_counter;
> > +   unsigned int rx_err_passive;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct can_frame *cf;
> > +   struct sk_buff *skb;
> > +   struct can_berr_counter bec;
> > +
> > +   /* propogate the error condition to the CAN stack */
> > +   skb = alloc_can_err_skb(dev, &cf);
> > +   if (unlikely(!skb))
> > +           return 0;
> > +
> > +   c_can_get_berr_counter(dev, &bec);
> > +   reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +   rx_err_passive = (reg_err_counter & ERR_CNT_RP_MASK) >>
> > +                           ERR_CNT_RP_SHIFT;
> > +
> > +   switch (error_type) {
> > +   case C_CAN_ERROR_WARNING:
> > +           /* error warning state */
> > +           priv->can.can_stats.error_warning++;
> > +           priv->can.state = CAN_STATE_ERROR_WARNING;
> > +           cf->can_id |= CAN_ERR_CRTL;
> > +           cf->data[1] = (bec.txerr > bec.rxerr) ?
> > +                   CAN_ERR_CRTL_TX_WARNING :
> > +                   CAN_ERR_CRTL_RX_WARNING;
> > +           cf->data[6] = bec.txerr;
> > +           cf->data[7] = bec.rxerr;
> > +
> > +           break;
> > +   case C_CAN_ERROR_PASSIVE:
> > +           /* error passive state */
> > +           priv->can.can_stats.error_passive++;
> > +           priv->can.state = CAN_STATE_ERROR_PASSIVE;
> > +           cf->can_id |= CAN_ERR_CRTL;
> > +           if (rx_err_passive)
> > +                   cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> > +           if (bec.txerr > 127)
> > +                   cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> > +
> > +           cf->data[6] = bec.txerr;
> > +           cf->data[7] = bec.rxerr;
> > +           break;
> > +   case C_CAN_BUS_OFF:
> > +           /* bus-off state */
> > +           priv->can.state = CAN_STATE_BUS_OFF;
> > +           cf->can_id |= CAN_ERR_BUSOFF;
> > +           /*
> > +            * disable all interrupts in bus-off mode to ensure that
> > +            * the CPU is not hogged down
> > +            */
> > +           c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +           can_bus_off(dev);
> > +           break;
> > +   default:
> > +           break;
> > +   }
> > +
> > +   netif_receive_skb(skb);
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += cf->can_dlc;
> > +
> > +   return 1;
> > +}
> > +
> > +static int c_can_handle_bus_err(struct net_device *dev,
> > +                           enum c_can_lec_type lec_type)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct can_frame *cf;
> > +   struct sk_buff *skb;
> > +
> > +   /*
> > +    * early exit if no lec update or no error.
> > +    * no lec update means that no CAN bus event has been detected
> > +    * since CPU wrote 0x7 value to status reg.
> > +    */
> > +   if (lec_type == LEC_UNUSED || lec_type == LEC_NO_ERROR)
> > +           return 0;
> > +
> > +   /* propogate the error condition to the CAN stack */
> > +   skb = alloc_can_err_skb(dev, &cf);
> > +   if (unlikely(!skb))
> > +           return 0;
> > +
> > +   /*
> > +    * check for 'last error code' which tells us the
> > +    * type of the last error to occur on the CAN bus
> > +    */
> > +
> > +   /* common for all type of bus errors */
> > +   priv->can.can_stats.bus_error++;
> > +   stats->rx_errors++;
> > +   cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > +   cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > +
> > +   switch (lec_type) {
> > +   case LEC_STUFF_ERROR:
> > +           netdev_dbg(dev, "stuff error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_STUFF;
> > +           break;
> > +   case LEC_FORM_ERROR:
> > +           netdev_dbg(dev, "form error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_FORM;
> > +           break;
> > +   case LEC_ACK_ERROR:
> > +           netdev_dbg(dev, "ack error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> > +                           CAN_ERR_PROT_LOC_ACK_DEL);
> > +           break;
> > +   case LEC_BIT1_ERROR:
> > +           netdev_dbg(dev, "bit1 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT1;
> > +           break;
> > +   case LEC_BIT0_ERROR:
> > +           netdev_dbg(dev, "bit0 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT0;
> > +           break;
> > +   case LEC_CRC_ERROR:
> > +           netdev_dbg(dev, "CRC error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > +                           CAN_ERR_PROT_LOC_CRC_DEL);
> > +           break;
> > +   default:
> > +           break;
> > +   }
> > +
> > +   /* set a `lec` value so that we can check for updates later */
> > +   priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> > +
> > +   netif_receive_skb(skb);
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += cf->can_dlc;
> > +
> > +   return 1;
> > +}
> > +
> > +static int c_can_poll(struct napi_struct *napi, int quota) {
> > +   u16 irqstatus;
> > +   int lec_type = 0;
> > +   int work_done = 0;
> > +   struct net_device *dev = napi->dev;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> > +   if (!irqstatus)
> > +           goto end;
> > +
> > +   /* status events have the highest priority */
> > +   if (irqstatus == STATUS_INTERRUPT) {
> > +           priv->current_status = priv->read_reg(priv,
> > +                                   &priv->regs->status);
> > +
> > +           /* handle Tx/Rx events */
> > +           if (priv->current_status & STATUS_TXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_TXOK);
> > +
> > +           if (priv->current_status & STATUS_RXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_RXOK);
> > +
> > +           /* handle state changes */
> > +           if ((priv->current_status & STATUS_EWARN) &&
> > +                           (!(priv->last_status & STATUS_EWARN))) {
> > +                   netdev_dbg(dev, "entered error warning state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_ERROR_WARNING);
> > +           }
> > +           if ((priv->current_status & STATUS_EPASS) &&
> > +                           (!(priv->last_status & STATUS_EPASS))) {
> > +                   netdev_dbg(dev, "entered error passive state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_ERROR_PASSIVE);
> > +           }
> > +           if ((priv->current_status & STATUS_BOFF) &&
> > +                           (!(priv->last_status & STATUS_BOFF))) {
> > +                   netdev_dbg(dev, "entered bus off state\n");
> > +                   work_done += c_can_handle_state_change(dev,
> > +                                           C_CAN_BUS_OFF);
> > +           }
> > +
> > +           /* handle bus recovery events */
> > +           if ((!(priv->current_status & STATUS_BOFF)) &&
> > +                           (priv->last_status & STATUS_BOFF)) {
> > +                   netdev_dbg(dev, "left bus off state\n");
> > +                   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +           }
> > +           if ((!(priv->current_status & STATUS_EPASS)) &&
> > +                           (priv->last_status & STATUS_EPASS)) {
> > +                   netdev_dbg(dev, "left error passive state\n");
> > +                   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +           }
> > +
> > +           priv->last_status = priv->current_status;
> > +
> > +           /* handle lec errors on the bus */
> > +           lec_type = c_can_has_and_handle_berr(priv);
> > +           if (lec_type)
> > +                   work_done += c_can_handle_bus_err(dev, lec_type);
> > +   } else if ((irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
> > +                   (irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
> > +           /* handle events corresponding to receive message objects
> */
> > +           work_done += c_can_do_rx_poll(dev, (quota - work_done));
> > +   } else if ((irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
> > +                   (irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
> > +           /* handle events corresponding to transmit message objects
> */
> > +           c_can_do_tx(dev);
> > +   }
> > +
> > +end:
> > +   if (work_done < quota) {
> > +           napi_complete(napi);
> > +           /* enable all IRQs */
> > +           c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +   }
> > +
> > +   return work_done;
> > +}
> > +
> > +static irqreturn_t c_can_isr(int irq, void *dev_id) {
> > +   u16 irqstatus;
> > +   struct net_device *dev = (struct net_device *)dev_id;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> > +   if (!irqstatus)
> > +           return IRQ_NONE;
> > +
> > +   /* disable all interrupts and schedule the NAPI */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +   napi_schedule(&priv->napi);
> > +
> > +   return IRQ_HANDLED;
> > +}
> > +
> > +static int c_can_open(struct net_device *dev) {
> > +   int err;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* open the can device */
> > +   err = open_candev(dev);
> > +   if (err) {
> > +           netdev_err(dev, "failed to open can device\n");
> > +           return err;
> > +   }
> > +
> > +   /* register interrupt handler */
> > +   err = request_irq(dev->irq, &c_can_isr, IRQF_SHARED, dev->name,
> > +                           dev);
> > +   if (err < 0) {
> > +           netdev_err(dev, "failed to request interrupt\n");
> > +           goto exit_irq_fail;
> > +   }
> > +
> > +   /* start the c_can controller */
> > +   c_can_start(dev);
> > +
> > +   napi_enable(&priv->napi);
> > +   netif_start_queue(dev);
> > +
> > +   return 0;
> > +
> > +exit_irq_fail:
> > +   close_candev(dev);
> > +   return err;
> > +}
> > +
> > +static int c_can_close(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   netif_stop_queue(dev);
> > +   napi_disable(&priv->napi);
> > +   c_can_stop(dev);
> > +   free_irq(dev->irq, dev);
> > +   close_candev(dev);
> > +
> > +   return 0;
> > +}
> > +
> > +struct net_device *alloc_c_can_dev(void) {
> > +   struct net_device *dev;
> > +   struct c_can_priv *priv;
> > +
> > +   dev = alloc_candev(sizeof(struct c_can_priv),
> C_CAN_MSG_OBJ_TX_NUM);
> > +   if (!dev)
> > +           return NULL;
> > +
> > +   priv = netdev_priv(dev);
> > +   netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> > +
> > +   priv->dev = dev;
> > +   priv->can.bittiming_const = &c_can_bittiming_const;
> > +   priv->can.do_set_bittiming = c_can_set_bittiming;
>
> Please move the set_bittiming to the _open() or the _start() function.
> See b156fd0483c8f18b3cc544d9c400fe454458e16a. So we can get rid of the
> can.do_set_bittiming sooner or later.

Ok. Will make this change in V7.

> > +   priv->can.do_set_mode = c_can_set_mode;
> > +   priv->can.do_get_berr_counter = c_can_get_berr_counter;
> > +   priv->can.ctrlmode_supported = CAN_CTRLMODE_ONE_SHOT |
> > +                                   CAN_CTRLMODE_LOOPBACK |
> > +                                   CAN_CTRLMODE_LISTENONLY |
> > +                                   CAN_CTRLMODE_BERR_REPORTING;
> > +
> > +   return dev;
> > +}
> > +EXPORT_SYMBOL_GPL(alloc_c_can_dev);
> > +
> > +void free_c_can_dev(struct net_device *dev) {
> > +   free_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(free_c_can_dev);
> > +
> > +static const struct net_device_ops c_can_netdev_ops = {
> > +   .ndo_open = c_can_open,
> > +   .ndo_stop = c_can_close,
> > +   .ndo_start_xmit = c_can_start_xmit,
> > +};
> > +
> > +int register_c_can_dev(struct net_device *dev) {
> > +   dev->flags |= IFF_ECHO; /* we support local echo */
> > +   dev->netdev_ops = &c_can_netdev_ops;
> > +
> > +   return register_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(register_c_can_dev);
> > +
> > +void unregister_c_can_dev(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   unregister_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(unregister_c_can_dev);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("CAN bus driver for
> > +Bosch C_CAN controller");
> > diff --git a/drivers/net/can/c_can/c_can.h
> > b/drivers/net/can/c_can/c_can.h new file mode 100644 index
> > 0000000..bd094e6
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -0,0 +1,230 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef C_CAN_H
> > +#define C_CAN_H
> > +
> > +/* control register */
> > +#define CONTROL_TEST               BIT(7)
> > +#define CONTROL_CCE                BIT(6)
> > +#define CONTROL_DISABLE_AR BIT(5)
> > +#define CONTROL_ENABLE_AR  (0 << 5)
> > +#define CONTROL_EIE                BIT(3)
> > +#define CONTROL_SIE                BIT(2)
> > +#define CONTROL_IE         BIT(1)
> > +#define CONTROL_INIT               BIT(0)
> > +
> > +/* test register */
> > +#define TEST_RX                    BIT(7)
> > +#define TEST_TX1           BIT(6)
> > +#define TEST_TX2           BIT(5)
> > +#define TEST_LBACK         BIT(4)
> > +#define TEST_SILENT                BIT(3)
> > +#define TEST_BASIC         BIT(2)
> > +
> > +/* status register */
> > +#define STATUS_BOFF                BIT(7)
> > +#define STATUS_EWARN               BIT(6)
> > +#define STATUS_EPASS               BIT(5)
> > +#define STATUS_RXOK                BIT(4)
> > +#define STATUS_TXOK                BIT(3)
> > +
> > +/* error counter register */
> > +#define ERR_CNT_TEC_MASK   0xff
> > +#define ERR_CNT_TEC_SHIFT  0
> > +#define ERR_CNT_REC_SHIFT  8
> > +#define ERR_CNT_REC_MASK   (0x7f << ERR_CNT_REC_SHIFT)
> > +#define ERR_CNT_RP_SHIFT   15
> > +#define ERR_CNT_RP_MASK            (0x1 << ERR_CNT_RP_SHIFT)
> > +
> > +/* bit-timing register */
> > +#define BTR_BRP_MASK               0x3f
> > +#define BTR_BRP_SHIFT              0
> > +#define BTR_SJW_SHIFT              6
> > +#define BTR_SJW_MASK               (0x3 << BTR_SJW_SHIFT)
> > +#define BTR_TSEG1_SHIFT            8
> > +#define BTR_TSEG1_MASK             (0xf << BTR_TSEG1_SHIFT)
> > +#define BTR_TSEG2_SHIFT            12
> > +#define BTR_TSEG2_MASK             (0x7 << BTR_TSEG2_SHIFT)
> > +
> > +/* brp extension register */
> > +#define BRP_EXT_BRPE_MASK  0x0f
> > +#define BRP_EXT_BRPE_SHIFT 0
> > +
> > +/* IFx command request */
> > +#define IF_COMR_BUSY               BIT(15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR         BIT(7)
> > +#define IF_COMM_MASK               BIT(6)
> > +#define IF_COMM_ARB                BIT(5)
> > +#define IF_COMM_CONTROL            BIT(4)
> > +#define IF_COMM_CLR_INT_PND        BIT(3)
> > +#define IF_COMM_TXRQST             BIT(2)
> > +#define IF_COMM_DATAA              BIT(1)
> > +#define IF_COMM_DATAB              BIT(0)
> > +#define IF_COMM_ALL                (IF_COMM_MASK | IF_COMM_ARB | \
> > +                           IF_COMM_CONTROL | IF_COMM_TXRQST | \
> > +                           IF_COMM_DATAA | IF_COMM_DATAB)
> > +
> > +/* IFx arbitration */
> > +#define IF_ARB_MSGVAL              BIT(15)
> > +#define IF_ARB_MSGXTD              BIT(14)
> > +#define IF_ARB_TRANSMIT            BIT(13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT            BIT(15)
> > +#define IF_MCONT_MSGLST            BIT(14)
> > +#define IF_MCONT_CLR_MSGLST        (0 << 14)
> > +#define IF_MCONT_INTPND            BIT(13)
> > +#define IF_MCONT_UMASK             BIT(12)
> > +#define IF_MCONT_TXIE              BIT(11)
> > +#define IF_MCONT_RXIE              BIT(10)
> > +#define IF_MCONT_RMTEN             BIT(9)
> > +#define IF_MCONT_TXRQST            BIT(8)
> > +#define IF_MCONT_EOB               BIT(7)
> > +#define IF_MCONT_DLC_MASK  0xf
> > +
> > +/*
> > + * IFx register masks:
> > + * allow easy operation on 16-bit registers when the
> > + * argument is 32-bit instead
> > + */
> > +#define IFX_WRITE_LOW_16BIT(x)     ((x) & 0xFFFF)
> > +#define IFX_WRITE_HIGH_16BIT(x)    (((x) & 0xFFFF0000) >> 16)
> > +
> > +/* message object split */
> > +#define C_CAN_NO_OF_OBJECTS        32
> > +#define C_CAN_MSG_OBJ_RX_NUM       16
> > +#define C_CAN_MSG_OBJ_TX_NUM       16
> > +
> > +#define C_CAN_MSG_OBJ_RX_FIRST     1
> > +#define C_CAN_MSG_OBJ_RX_LAST      (C_CAN_MSG_OBJ_RX_FIRST + \
> > +                           C_CAN_MSG_OBJ_RX_NUM - 1)
> > +
> > +#define C_CAN_MSG_OBJ_TX_FIRST     (C_CAN_MSG_OBJ_RX_LAST + 1)
> > +#define C_CAN_MSG_OBJ_TX_LAST      (C_CAN_MSG_OBJ_TX_FIRST + \
> > +                           C_CAN_MSG_OBJ_TX_NUM - 1)
> > +
> > +#define C_CAN_MSG_OBJ_RX_SPLIT     9
> > +#define C_CAN_MSG_RX_LOW_LAST      (C_CAN_MSG_OBJ_RX_SPLIT - 1)
> > +
> > +#define C_CAN_NEXT_MSG_OBJ_MASK    (C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS        0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT   0x8000
> > +
> > +/* global interrupt masks */
> > +#define ENABLE_ALL_INTERRUPTS      1
> > +#define DISABLE_ALL_INTERRUPTS     0
> > +
> > +/* minimum timeout for checking BUSY status */
> > +#define MIN_TIMEOUT_VALUE  6
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT  C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > +   u16 com_req;
> > +   u16 com_mask;
> > +   u16 mask1;
> > +   u16 mask2;
> > +   u16 arb1;
> > +   u16 arb2;
> > +   u16 msg_cntrl;
> > +   u16 data[4];
> > +   u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > +   u16 control;
> > +   u16 status;
> > +   u16 err_cnt;
> > +   u16 btr;
> > +   u16 interrupt;
> > +   u16 test;
> > +   u16 brp_ext;
> > +   u16 _reserved1;
> > +   struct c_can_if_regs ifregs[2]; /* [0] = IF1 and [1] = IF2 */
> > +   u16 _reserved2[8];
> > +   u16 txrqst1;
> > +   u16 txrqst2;
> > +   u16 _reserved3[6];
> > +   u16 newdat1;
> > +   u16 newdat2;
> > +   u16 _reserved4[6];
> > +   u16 intpnd1;
> > +   u16 intpnd2;
> > +   u16 _reserved5[6];
> > +   u16 msgval1;
> > +   u16 msgval2;
> > +   u16 _reserved6[6];
> > +};
> > +
> > +/* c_can lec values */
> > +enum c_can_lec_type {
> > +   LEC_NO_ERROR = 0,
> > +   LEC_STUFF_ERROR,
> > +   LEC_FORM_ERROR,
> > +   LEC_ACK_ERROR,
> > +   LEC_BIT1_ERROR,
> > +   LEC_BIT0_ERROR,
> > +   LEC_CRC_ERROR,
> > +   LEC_UNUSED,
> > +};
> > +
> > +/*
> > + * c_can error types:
> > + * Bus errors (BUS_OFF, ERROR_WARNING, ERROR_PASSIVE) are supported
> > +*/ enum c_can_bus_error_types {
> > +   C_CAN_NO_ERROR = 0,
> > +   C_CAN_BUS_OFF,
> > +   C_CAN_ERROR_WARNING,
> > +   C_CAN_ERROR_PASSIVE,
> > +};
>
> nitpick: are the defines, enums and structs needed in more than one c
> file? If not, please move them into the c-file where they are used.

Well most of the strcuts/defines are useful in both `c_can.c` and
`c_can_platform.c`, but I will explore how to separate the rest in
the respective c-files. But inititally we agreed to a *sja1000* like
approach and hence this placement in h-file.

> > +
> > +/* c_can private data structure */
> > +struct c_can_priv {
> > +   struct can_priv can;    /* must be the first member */
> > +   struct napi_struct napi;
> > +   struct net_device *dev;
> > +   int tx_object;
> > +   int current_status;
> > +   int last_status;
> > +   u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> > +   void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> > +   struct c_can_regs __iomem *regs;
> > +   unsigned long irq_flags; /* for request_irq() */
> > +   unsigned int tx_next;
> > +   unsigned int tx_echo;
> > +   void *priv;             /* for board-specific data */
> > +};
> > +
> > +struct net_device *alloc_c_can_dev(void); void free_c_can_dev(struct
> > +net_device *dev); int register_c_can_dev(struct net_device *dev);
> > +void unregister_c_can_dev(struct net_device *dev);
> > +
> > +#endif /* C_CAN_H */
> > diff --git a/drivers/net/can/c_can/c_can_platform.c
> > b/drivers/net/can/c_can/c_can_platform.c
> > new file mode 100644
> > index 0000000..0fc314e
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -0,0 +1,207 @@
> > +/*
> > + * Platform CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/if_arp.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/list.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +
> > +#include <linux/can/dev.h>
> > +
> > +#include "c_can.h"
> > +
> > +/*
> > + * 16-bit c_can registers can be arranged differently in the memory
> > + * architecture of different implementations. For example: 16-bit
> > + * registers can be aligned to a 16-bit boundary or 32-bit boundary
> etc.
> > + * Handle the same by providing a common read/write interface.
> > + */
> > +static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv
> *priv,
> > +                                           void *reg)
> > +{
> > +   return readw(reg);
> > +}
> > +
> > +static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv
> *priv,
> > +                                           void *reg, u16 val)
> > +{
> > +   writew(val, reg);
> > +}
> > +
> > +static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > +                                           void *reg)
> > +{
> > +   return readw(reg + (long)reg - (long)priv->regs); }
> > +
> > +static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > +                                           void *reg, u16 val)
> > +{
> > +   writew(val, reg + (long)reg - (long)priv->regs); }
> > +
> > +static int __devinit c_can_plat_probe(struct platform_device *pdev)
> {
> > +   int ret;
> > +   void __iomem *addr;
> > +   struct net_device *dev;
> > +   struct c_can_priv *priv;
> > +   struct resource *mem, *irq;
> > +   struct clk *clk;
> > +
> > +   /* get the appropriate clk */
> > +   clk = clk_get(&pdev->dev, NULL);
> > +   if (IS_ERR(clk)) {
> > +           dev_err(&pdev->dev, "no clock defined\n");
> > +           ret = -ENODEV;
> > +           goto exit;
> > +   }
> > +
> > +   /* get the platform data */
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +   if (!mem || (irq <= 0)) {
> > +           ret = -ENODEV;
> > +           goto exit_free_clk;
> > +   }
> > +
> > +   if (!request_mem_region(mem->start, resource_size(mem),
> > +                           KBUILD_MODNAME)) {
> > +           dev_err(&pdev->dev, "resource unavailable\n");
> > +           ret = -ENODEV;
> > +           goto exit_free_clk;
> > +   }
> > +
> > +   addr = ioremap(mem->start, resource_size(mem));
> > +   if (!addr) {
> > +           dev_err(&pdev->dev, "failed to map can port\n");
> > +           ret = -ENOMEM;
> > +           goto exit_release_mem;
> > +   }
> > +
> > +   /* allocate the c_can device */
> > +   dev = alloc_c_can_dev();
> > +   if (!dev) {
> > +           ret = -ENOMEM;
> > +           goto exit_iounmap;
> > +   }
> > +
> > +   priv = netdev_priv(dev);
> > +
> > +   dev->irq = irq->start;
> > +   priv->regs = addr;
> > +   priv->can.clock.freq = clk_get_rate(clk);
> > +   priv->priv = clk;
> > +
> > +   switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> > +   case IORESOURCE_MEM_32BIT:
> > +           priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> > +           priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> > +           break;
> > +   case IORESOURCE_MEM_16BIT:
> > +   default:
> > +           priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > +           priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > +           break;
> > +   }
> > +
> > +   platform_set_drvdata(pdev, dev);
> > +   SET_NETDEV_DEV(dev, &pdev->dev);
> > +
> > +   ret = register_c_can_dev(dev);
> > +   if (ret) {
> > +           dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> > +                   KBUILD_MODNAME, ret);
> > +           goto exit_free_device;
> > +   }
> > +
> > +   dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> > +            KBUILD_MODNAME, priv->regs, dev->irq);
> > +   return 0;
> > +
> > +exit_free_device:
> > +   platform_set_drvdata(pdev, NULL);
> > +   free_c_can_dev(dev);
> > +exit_iounmap:
> > +   iounmap(addr);
> > +exit_release_mem:
> > +   release_mem_region(mem->start, resource_size(mem));
> > +exit_free_clk:
> > +   clk_put(clk);
> > +exit:
> > +   dev_err(&pdev->dev, "probe failed\n");
> > +
> > +   return ret;
> > +}
> > +
> > +static int __devexit c_can_plat_remove(struct platform_device *pdev)
> > +{
> > +   struct net_device *dev = platform_get_drvdata(pdev);
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct resource *mem;
> > +
> > +   unregister_c_can_dev(dev);
> > +   platform_set_drvdata(pdev, NULL);
> > +
> > +   free_c_can_dev(dev);
> > +   iounmap(priv->regs);
> > +
> > +   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   release_mem_region(mem->start, resource_size(mem));
> > +
> > +   clk_put(priv->priv);
> > +
> > +   return 0;
> > +}
> > +
> > +static struct platform_driver c_can_plat_driver = {
> > +   .driver = {
> > +           .name = KBUILD_MODNAME,
> > +           .owner = THIS_MODULE,
> > +   },
> > +   .probe = c_can_plat_probe,
> > +   .remove = __devexit_p(c_can_plat_remove), };
> > +
> > +static int __init c_can_plat_init(void) {
> > +   return platform_driver_register(&c_can_plat_driver);
> > +}
> > +module_init(c_can_plat_init);
> > +
> > +static void __exit c_can_plat_exit(void) {
> > +   platform_driver_unregister(&c_can_plat_driver);
> > +}
> > +module_exit(c_can_plat_exit);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("Platform CAN bus
> driver
> > +for Bosch C_CAN controller");
>
> regards, Marc

Regards,
Bhupesh

^ permalink raw reply

* Hardware bridging for Distributed Switch Architecture (DSA)
From: Mickael Chazaux @ 2011-02-09 10:30 UTC (permalink / raw)
  To: netdev, buytenh

Hi,

I'm interested in working on hardware bridging support for the
Distributed Switch Architecture (DSA).

I have a few questions about this patch [1] :

"(While the patch in itself works, the implementation is ugly, and I
don't see it being merged upstream in this form any time soon"

Why this? What needs to be done to clean it up, and have it in shape?

"While conceptually I think this is the way this should work (i.e. no
workflow changes for the system administrator) "

I have worked a bit on this subject too and concluded to the same idea.

" the code shouldn't be hooking directly into the bridge code at all.
Ideally, it should just be using a netlink interface providing this info:
- bridge port group creation/destruction
- adding/removing an interface to/from a bridge port group
- changing the STP state of an interface in a bridge port group"

Doesn't brctl already uses netlink to configure the bridge? Could you expand
a bit more on this?

I like this approach of automatically hardware-bridge ports if possible when
adding them to a bridge. Do you mean putting a specific API in netlink
for doing
the hardware bridging stuff ?

Regards,

Mickael

[1] http://patchwork.ozlabs.org/patch/16578/

^ permalink raw reply


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