* 2.6.37-rc6-git4: Reported regressions from 2.6.36
From: Rafael J. Wysocki @ 2010-12-19 12:28 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Linux SCSI List, Linux ACPI, Network Development,
Linux Wireless List, DRI, Florian Mickler, Andrew Morton,
Kernel Testers List, Linus Torvalds, Linux PM List,
Maciej Rutecki
This message contains a list of some regressions from 2.6.36,
for which there are no fixes in the mainline known to the tracking team.
If any of them have been fixed already, please let us know.
If you know of any other unresolved regressions from 2.6.36, please let us
know either and we'll add them to the list. Also, please let us know
if any of the entries below are invalid.
Each entry from the list will be sent additionally in an automatic reply
to this message with CCs to the people involved in reporting and handling
the issue.
Listed regressions statistics:
Date Total Pending Unresolved
----------------------------------------
2010-12-19 73 28 24
2010-12-03 55 25 19
2010-11-19 39 29 25
Unresolved regressions
----------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=25012
Subject : BUG: i915 causes NULL pointer dereference in 2.6.37-rc5-git4
Submitter : Tõnu Raitviir <jussuf@linux.ee>
Date : 2010-12-15 12:48 (5 days old)
Message-ID : <alpine.DEB.2.00.1012151238570.4797@jbbyvx.ohzcpyho.rr>
References : http://www.spinics.net/lists/dri-devel/msg06282.html
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24892
Subject : Wireless warning, system locks up
Submitter : Heinz Diehl <htd@fancy-poultry.org>
Date : 2010-12-14 15:16 (6 days old)
Handled-By : John W. Linville <linville@tuxdriver.com>
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24882
Subject : PM/Hibernate: Memory corruption patch introduces regression (2.6.36.2)
Submitter : <akwatts@ymail.com>
Date : 2010-12-14 04:00 (6 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24822
Subject : Embedded DisplayPort is detected wrongly on HP ProBook 5320m
Submitter : Takashi Iwai <tiwai@suse.de>
Date : 2010-12-13 11:09 (7 days old)
Handled-By : Chris Wilson <chris@chris-wilson.co.uk>
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24772
Subject : Crash with btrfs rootfs on dm-crypt [ kernel BUG at fs/btrfs/inode.c:806! ] on linux 2.6.37-rc5
Submitter : Fabio Comolli <fabio.comolli@gmail.com>
Date : 2010-12-10 20:30 (10 days old)
Message-ID : <AANLkTi=j9zsaYNcu=NgGV=HfE-3rNHzVswov8VrgwjQp@mail.gmail.com>
References : http://marc.info/?l=linux-kernel&m=129201308706568&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24762
Subject : BUG at perf_ctx_adjust_freq (kernel/perf_event.c:1582)
Submitter : Chris Wilson <chris@chris-wilson.co.uk>
Date : 2010-12-10 12:00 (10 days old)
Message-ID : <c6d829$pqibha@fmsmga001.fm.intel.com>
References : http://marc.info/?l=linux-kernel&m=129198247531612&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24722
Subject : Disconnecting my USB mouse hangs the machine and issues kernel warning
Submitter : Heinz Diehl <htd@fancy-poultry.org>
Date : 2010-12-12 12:42 (8 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24592
Subject : 2.6.37-rc5: NULL pointer oops in selinux_socket_unix_stream_connect
Submitter : Jeremy Fitzhardinge <jeremy@goop.org>
Date : 2010-12-08 21:09 (12 days old)
Message-ID : <4CFFF3F3.90100@goop.org>
References : http://marc.info/?l=linux-kernel&m=129184256629712&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24582
Subject : Kernel Oops at tty_buffer_request_room when using pppd program (2.6.37-rc4)
Submitter : baoyb <baoyb@avit.org.cn>
Date : 2010-12-08 13:55 (12 days old)
Message-ID : <EF6DDE218DB34702B1FA84D6CD7EA771@baoyb>
References : http://marc.info/?l=linux-kernel&m=129181763525738&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24372
Subject : kdump broken on 2.6.37-rc4
Submitter : Stanislaw Gruszka <sgruszka@redhat.com>
Date : 2010-12-03 11:16 (17 days old)
Message-ID : <20101203111623.GA2741@redhat.com>
References : http://marc.info/?l=linux-kernel&m=129137502323003&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24362
Subject : perf hw in kexeced kernel broken in tip
Submitter : Yinghai Lu <yinghai@kernel.org>
Date : 2010-12-01 8:00 (19 days old)
First-Bad-Commit: http://git.kernel.org/linus/33c6d6a7ad0ffab9b1b15f8e4107a2af072a05a0
Message-ID : <4CF60095.1020900@kernel.org>
References : http://marc.info/?l=linux-kernel&m=129119055922065&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24272
Subject : iotop reports insane per-process disk read/write statistics
Submitter : Brian Rogers <brian@xyzw.org>
Date : 2010-12-03 12:00 (17 days old)
First-Bad-Commit: http://git.kernel.org/linus/85893120699f8bae8caa12a8ee18ab5fceac978e
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24142
Subject : Crash at boot in udev while accessing bt848 device in 2.6.37-rc*
Submitter : Christian Casteyde <casteyde.christian@free.fr>
Date : 2010-11-30 20:50 (20 days old)
References : https://bugzilla.kernel.org/show_bug.cgi?id=24602
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=23902
Subject : [BUG] 2.6.37-rc3 massive interactivity regression on ARM
Submitter : Mikael Pettersson <mikpe@it.uu.se>
Date : 2010-11-27 15:16 (23 days old)
First-Bad-Commit: http://git.kernel.org/linus/305e6835e05513406fa12820e40e4a8ecb63743c
Message-ID : <<19697.8378.717761.236202@pilspetsen.it.uu.se>>
References : http://marc.info/?l=linux-kernel&m=129087098911837&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=23472
Subject : 2.6.37-rc2 vs. 2.6.36 laptop backlight changes?
Submitter : Patrick Schaaf <netdev@bof.de>
Date : 2010-11-17 13:41 (33 days old)
Message-ID : <1290001262.5727.2.camel@lat1>
References : http://marc.info/?l=linux-kernel&m=129000127920912&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=23102
Subject : [bisected] i915 regression in post 2.6.36 kernels
Submitter : Johannes Hirte <johannes.hirte@fem.tu-ilmenau.de>
Date : 2010-11-10 7:02 (40 days old)
Message-ID : <201011100802.20332.johannes.hirte@fem.tu-ilmenau.de>
References : http://marc.info/?l=linux-kernel&m=128937310017057&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22942
Subject : [2.6.37-rc1, OOM] virtblk: OOM in do_virtblk_request()
Submitter : Dave Chinner <david@fromorbit.com>
Date : 2010-11-05 1:30 (45 days old)
Message-ID : <20101105013003.GE13830@dastard>
References : http://marc.info/?l=linux-kernel&m=128892062917641&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22912
Subject : spi_lm70llp module crash on unload (2.6.37-rc1)
Submitter : Randy Dunlap <randy.dunlap@oracle.com>
Date : 2010-11-05 0:16 (45 days old)
Message-ID : <20101104171620.00d8c95d.randy.dunlap@oracle.com>
References : http://marc.info/?l=linux-kernel&m=128891627913647&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22882
Subject : (2.6.37-rc1) amd64-agp module crashed on second load
Submitter : Randy Dunlap <randy.dunlap@oracle.com>
Date : 2010-11-05 0:13 (45 days old)
Message-ID : <20101104171333.fea1f498.randy.dunlap@oracle.com>
References : http://marc.info/?l=linux-kernel&m=128891605213447&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22812
Subject : kernel oops on 2.6.37-rc1
Submitter : Andrew <atswartz@gmail.com>
Date : 2010-11-12 16:05 (38 days old)
First-Bad-Commit: http://git.kernel.org/linus/a68c439b1966c91f0ef474e2bf275d6792312726
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22642
Subject : 2.6.37-rc1: Disk takes 10 seconds to resume - MacBook2,1
Submitter : Tobias <devnull@plzk.org>
Date : 2010-11-10 19:33 (40 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22562
Subject : Regression in 2.6.37-rc1 - logs spammed with "unable to enumerate USB port" - bisected to commit 3df7169e
Submitter : Larry Finger <Larry.Finger@lwfinger.net>
Date : 2010-11-02 22:32 (48 days old)
First-Bad-Commit: http://git.kernel.org/linus/3df7169e73fc1d71a39cffeacc969f6840cdf52b
Message-ID : <4CD09166.4060202@lwfinger.net>
References : http://marc.info/?l=linux-kernel&m=128873713207906&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22542
Subject : [2.6.37-rc1] drm:i195 errors
Submitter : Paul Rolland <rol@witbe.net>
Date : 2010-11-02 14:58 (48 days old)
Message-ID : <20101102155813.09cb2c6e@tux.DEF.witbe.net>
References : http://marc.info/?l=linux-kernel&m=128870991628970&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22472
Subject : vga_switcheroo fails to switch from intel to ati
Submitter : Radu Andries <admiral0@tuxfamily.org>
Date : 2010-11-08 16:46 (42 days old)
Regressions with patches
------------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24602
Subject : modprobe bttv crashes system
Submitter : Sergej Pupykin <pupykin.s@gmail.com>
Date : 2010-12-09 21:48 (11 days old)
References : https://bugzilla.kernel.org/show_bug.cgi?id=24142
Patch : https://patchwork.kernel.org/patch/414671/
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24462
Subject : r600: spread spectrum: flickering screen (bisected)
Submitter : Luca Tettamanti <kronos.it@gmail.com>
Date : 2010-12-08 16:18 (12 days old)
First-Bad-Commit: http://git.kernel.org/linus/ba032a58d1f320039e7850fb6e8651695c1aa571
Handled-By : Alex Deucher <alexdeucher@gmail.com>
Patch : https://bugzilla.kernel.org/attachment.cgi?id=39372
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22672
Subject : Regression in 2.6.37-rc1 for Intel 945 Graphics Adapter - bisected to commit e9e331a
Submitter : Larry Finger <Larry.Finger@lwfinger.net>
Date : 2010-11-11 01:56 (39 days old)
References : https://bugs.freedesktop.org/show_bug.cgi?id=31803
http://marc.info/?l=linux-kernel&m=128944001311444&w=2
http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg02235.html
Patch : https://patchwork.kernel.org/patch/359472/
https://patchwork.kernel.org/patch/359502/
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22662
Subject : divide error in select_task_rq_fair()
Submitter : Myron Stowe <myron.stowe@hp.com>
Date : 2010-11-10 23:58 (40 days old)
Patch : http://lkml.org/lkml/2010/11/13/176
http://lkml.org/lkml/2010/11/13/181
For details, please visit the bug entries and follow the links given in
references.
As you can see, there is a Bugzilla entry for each of the listed regressions.
There also is a Bugzilla entry used for tracking the regressions from 2.6.36,
unresolved as well as resolved, at:
http://bugzilla.kernel.org/show_bug.cgi?id=21782
Please let the tracking team know if there are any Bugzilla entries that
should be added to the list in there.
Thanks!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* 2.6.37-rc6-git4: Reported regressions 2.6.35 -> 2.6.36
From: Rafael J. Wysocki @ 2010-12-19 12:41 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Maciej Rutecki, Florian Mickler, Andrew Morton, Linus Torvalds,
Kernel Testers List, Network Development, Linux ACPI,
Linux PM List, Linux SCSI List, Linux Wireless List, DRI
This message contains a list of some post-2.6.35 regressions introduced before
2.6.36, for which there are no fixes in the mainline known to the tracking team.
If any of them have been fixed already, please let us know.
If you know of any other unresolved post-2.6.35 regressions, please let us know
either and we'll add them to the list. Also, please let us know if any
of the entries below are invalid.
Each entry from the list will be sent additionally in an automatic reply to
this message with CCs to the people involved in reporting and handling the
issue.
Listed regressions statistics:
Date Total Pending Unresolved
----------------------------------------
2010-12-19 98 28 23
2010-12-05 95 34 31
2010-11-19 92 38 34
2010-10-17 70 27 27
2010-10-10 56 16 15
2010-10-03 52 16 14
2010-09-26 46 15 13
2010-09-20 38 15 15
2010-09-12 28 14 13
2010-08-30 21 16 15
Unresolved regressions
----------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24752
Subject : Random crashes easily reproducible with make -j5 - intel i915 - kernel 2.6.36 on intel/nvidia hybrid graphics machine
Submitter : Giacomo <delleceste-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-12-10 8:57 (10 days old)
Message-ID : <AANLkTimkQM94u9iz7FVVjehB0mwDwfkNwKhF2F2tYq-r-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=129197146619176&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24392
Subject : AGP aperture disabled, worked in 2.6.35
Submitter : Stephen Kitt <steve-HEvo97dlh4E@public.gmane.org>
Date : 2010-12-06 06:31 (14 days old)
First-Bad-Commit: http://git.kernel.org/linus/96576a9e1a0cdb8a43d3af5846be0948f52b4460
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24202
Subject : [830] drm:intel_prepare_page_flip, *ERROR* Prepared flip multiple times
Submitter : mkkot <marcin2006-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-12-02 14:10 (18 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=24022
Subject : wireless no longer works after 1st update of 10.10 [rtl819xE:ERR in init_firmware()]
Submitter : njin <marconifabio-osBHMS06NAwBXFe83j6qeQ@public.gmane.org>
Date : 2010-11-29 19:49 (21 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=23812
Subject : HAL does not provide battery information on RHEL5 and CentOS-5
Submitter : Dag Wieers <dag-TpjQRECdbeTQT0dZR+AlfA@public.gmane.org>
Date : 2010-11-26 18:08 (24 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=23302
Subject : alsa stops working after one or more hibernate or suspend cycles
Submitter : Werner Lemberg <wl-mXXj517/zsQ@public.gmane.org>
Date : 2010-11-19 16:21 (31 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22842
Subject : iwl3945 suddenly stops working
Submitter : Felipe Contreras <felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-11-14 11:14 (36 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22782
Subject : 2.6.36: general protection fault during lockfs lockspace removal
Submitter : nik-Jp3n8lUXroTtwjQa/ONI9g@public.gmane.org <nik-Jp3n8lUXroTtwjQa/ONI9g@public.gmane.org>
Date : 2010-11-12 12:05 (38 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22172
Subject : alsa-util.c: snd_pcm_avail_delay() returned strange values: delay 0 is less than avail 32
Submitter : Tobias <devnull-fBT1nhYaLZ4@public.gmane.org>
Date : 2010-11-06 09:33 (44 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=22092
Subject : Kernel v2.6.36 trouble on USB disconnect
Submitter : Ketil Froyn <ketil-wcohQK4BhvFBDLzU/O5InQ@public.gmane.org>
Date : 2010-10-29 8:05 (52 days old)
Message-ID : <AANLkTik5qVxkEGVAA1PSOGk2KTW+ekHpSwttsQEWzWj+-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128833956503607&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=21662
Subject : 2.6.35->2.6.36 regression, vanilla kernel panic, ppp or hrtimers crashing
Submitter : Denys Fedoryshchenko <nuclearcat-03OYUmBsc8OBik42HM7KXg@public.gmane.org>
Date : 2010-10-25 9:22 (56 days old)
Message-ID : <201010251222.37191.nuclearcat-03OYUmBsc8OBik42HM7KXg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128799855826011&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=21652
Subject : several problems with intel graphics since 2.6.36
Submitter : Norbert Preining <preining-DX+603jRYB8@public.gmane.org>
Date : 2010-10-27 14:32 (54 days old)
Message-ID : <20101027143252.GA8676-DqSSrKF0TaySnEC3TeqHn5dqbFPxfnh/@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128818998630241&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=21402
Subject : [KVM] Noacpi Windows guest can not boot up on 32bit KVM host
Submitter : xudong <xudong.hao-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date : 2010-10-29 03:01 (52 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20332
Subject : [LogFS] [2.6.36-rc7] Kernel BUG at lib/btree.c:465!
Submitter : Prasad Joshi <prasadjoshi124-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-10-12 18:56 (69 days old)
Message-ID : <AANLkTimAbCZNhLQ5nADUiAC+7JpAeJBEmjFwdxyZ-FxO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128690910501830&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20322
Subject : 2.6.36-rc7: inconsistent lock state: inconsistent {IN-RECLAIM_FS-R} -> {RECLAIM_FS-ON-W} usage.
Submitter : Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date : 2010-10-11 20:10 (70 days old)
Message-ID : <20101011201007.GA29707-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128682782828453&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20232
Subject : kworker consumes ~100% CPU on HP Elitebook 8540w running 2.6.36_rc6-git4
Submitter : Ozan Caglayan <ozan-caicS1wCkhO6A22drWdTBw@public.gmane.org>
Date : 2010-10-13 06:13 (68 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=19632
Subject : 2.6.36-rc6: modprobe Not tainted warning
Submitter : Heinz Diehl <htd-iEI8Y0CNJBYdnm+yROfE0A@public.gmane.org>
Date : 2010-09-30 18:25 (81 days old)
Message-ID : <20100930182516.GA15089-iEI8Y0CNJBYdnm+yROfE0A@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128587114004680&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=19392
Subject : WARNING: at drivers/net/wireless/ath/ath5k/base.c:3475 ath5k_bss_info_changed+0x44/0x168 [ath5k]()
Submitter : Justin Mattock <justinmattock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-09-28 22:30 (83 days old)
Message-ID : <<AANLkTim5WCGKPvEkOkO_YnMF9pg8mvLfQoFBNUFpfa_k-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>>
References : http://marc.info/?l=linux-kernel&m=128571307018635&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=19372
Subject : 2.6.36-rc6: WARNING: at drivers/gpu/drm/radeon/radeon_fence.c:235 radeon_fence_wait+0x35a/0x3c0
Submitter : Alexey Dobriyan <adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-09-29 21:29 (82 days old)
Message-ID : <20100929212923.GA5578-y0M6fkzdUYllgR+Ck+lCww@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128579579400315&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=19052
Subject : 2.6.36-rc5-git1 -- [drm:i915_report_and_clear_eir] *ERROR* EIR stuck: 0x00000010, masking
Submitter : Miles Lane <miles.lane-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-09-22 23:47 (89 days old)
Message-ID : <AANLkTikWQjUQjFJU9MO1+XbSLAEE-GARz+S+Dz2Fgu4h-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128519926626322&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=17121
Subject : Two blank rectangles more than 10 cm long when booting
Submitter : Eric Valette <eric.valette-GANU6spQydw@public.gmane.org>
Date : 2010-08-26 17:24 (116 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=17061
Subject : 2.6.36-rc1 on zaurus: bluetooth regression
Submitter : Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Date : 2010-08-21 15:24 (121 days old)
Message-ID : <20100821152445.GA1536-+ZI9xUNit7I@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128240433828087&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16951
Subject : hackbench regression with 2.6.36-rc1
Submitter : Zhang, Yanmin <yanmin_zhang-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Date : 2010-08-18 6:18 (124 days old)
Message-ID : <1282112318.21202.8.camel-sz7BYL/Y5Hu/P+R7jlPCFVaTQe2KTcn/@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128211235904910&w=2
Regressions with patches
------------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=21092
Subject : Kernel 2.6.36 Bug during quotaon on reiserfs
Submitter : <markus.gapp-hi6Y0CQ0nG0@public.gmane.org>
Date : 2010-10-24 16:57 (57 days old)
Handled-By : Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Patch : https://bugzilla.kernel.org/attachment.cgi?id=35292
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20462
Subject : 2.6.36-rc7-git2 - panic/GPF: e1000e/vlans?
Submitter : Nikola Ciprich <extmaillist-Jp3n8lUXroTtwjQa/ONI9g@public.gmane.org>
Date : 2010-10-15 7:10 (66 days old)
Message-ID : <20101015071008.GA8714-xTMdSLfc3Wpi51D5yjT6kKVXKuFTiq87@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128712984831303&w=2
Handled-By : Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Patch : http://www.spinics.net/lists/netdev/msg146227.html
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20342
Subject : [LogFS] [2.6.36-rc7] Deadlock in logfs_get_wblocks, hold and wait on same lock super->s_write_mutex
Submitter : Prasad Joshi <prasadjoshi124-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-10-13 9:49 (68 days old)
Message-ID : <AANLkTinvsMxTxEbDEFmb5M-6fYjdRvErU==Zs7+qANkV-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128696335024718&w=2
Patch : https://patchwork.kernel.org/patch/328682/
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=20162
Subject : [LogFS][2.6.36.rc7+] Kernel BUG at readwrite.c:1193
Submitter : Prasad Joshi <prasadjoshi124-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date : 2010-10-10 17:44 (71 days old)
Message-ID : <AANLkTi=JkcuWBPo+X-i+9o-BJFVqjea1J3e=Mr=HvAWF-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
References : http://marc.info/?l=linux-kernel&m=128673196203340&w=2
Handled-By : Prasad Gajanan Joshi <prasadjoshi124-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Patch : https://bugzilla.kernel.org/show_bug.cgi?id=20162#c1
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16971
Subject : qla4xxx compile failure on 32-bit PowerPC: missing readq and writeq
Submitter : Meelis Roos <mroos-Y27EyoLml9s@public.gmane.org>
Date : 2010-08-19 21:03 (123 days old)
Message-ID : <<<alpine.SOC.1.00.1008192359310.19654-ptEonEWSGqKptlylMvRsHA@public.gmane.org>>>
References : http://marc.info/?l=linux-kernel&m=128225184900892&w=2
Patch : http://marc.info/?l=linux-scsi&m=128590267608876&w=2
For details, please visit the bug entries and follow the links given in
references.
As you can see, there is a Bugzilla entry for each of the listed regressions.
There also is a Bugzilla entry used for tracking the regressions introduced
between 2.6.35 and 2.6.36, unresolved as well as resolved, at:
http://bugzilla.kernel.org/show_bug.cgi?id=16444
Please let the tracking team know if there are any Bugzilla entries that
should be added to the list in there.
Thanks!
^ permalink raw reply
* [PATCH] net: Add USB PID for new MOSCHIP USB ethernet controller MCS7832 variant
From: Andreas Mohr @ 2010-12-19 15:42 UTC (permalink / raw)
To: David S. Miller; +Cc: Arnd Bergmann, dhollis, Phil Chang, netdev, linux-kernel
In-Reply-To: <20101130200737.GA5185@rhlx01.hs-esslingen.de>
Due to active notification of the new MCS7832 version by the manufacturer
(Mr. Milton; thanks!) -- quote: "functionality same as MCS7830",
I'm now submitting this patch (on -rc6), intended for networking.git and -stable.
- add MCS7832 USB PID to be able to support this new device variant, too
- add related descriptions
Signed-off-by: Andreas Mohr <andi@lisas.de>
Cc: stable@kernel.org
---
Patch created, "semi"-tested (via my existing MCS7830 only),
checkpatch.pl'd.
GIT history seems clean, should thus apply easily.
Took longer (it's that time of the year again).
Thanks!
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index a6281e3..b701f59 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -1,5 +1,5 @@
/*
- * MOSCHIP MCS7830 based USB 2.0 Ethernet Devices
+ * MOSCHIP MCS7830 based (7730/7830/7832) USB 2.0 Ethernet Devices
*
* based on usbnet.c, asix.c and the vendor provided mcs7830 driver
*
@@ -11,6 +11,9 @@
*
* Definitions gathered from MOSCHIP, Data Sheet_7830DA.pdf (thanks!).
*
+ * 2010-12-19: add 7832 USB PID ("functionality same as MCS7830"),
+ * per active notification by manufacturer
+ *
* TODO:
* - support HIF_REG_CONFIG_SLEEPMODE/HIF_REG_CONFIG_TXENABLE (via autopm?)
* - implement ethtool_ops get_pauseparam/set_pauseparam
@@ -60,6 +63,7 @@
#define MCS7830_MAX_MCAST 64
#define MCS7830_VENDOR_ID 0x9710
+#define MCS7832_PRODUCT_ID 0x7832
#define MCS7830_PRODUCT_ID 0x7830
#define MCS7730_PRODUCT_ID 0x7730
@@ -626,7 +630,7 @@ static int mcs7830_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
}
static const struct driver_info moschip_info = {
- .description = "MOSCHIP 7830/7730 usb-NET adapter",
+ .description = "MOSCHIP 7830/7832/7730 usb-NET adapter",
.bind = mcs7830_bind,
.rx_fixup = mcs7830_rx_fixup,
.flags = FLAG_ETHER,
@@ -645,6 +649,10 @@ static const struct driver_info sitecom_info = {
static const struct usb_device_id products[] = {
{
+ USB_DEVICE(MCS7830_VENDOR_ID, MCS7832_PRODUCT_ID),
+ .driver_info = (unsigned long) &moschip_info,
+ },
+ {
USB_DEVICE(MCS7830_VENDOR_ID, MCS7830_PRODUCT_ID),
.driver_info = (unsigned long) &moschip_info,
},
^ permalink raw reply related
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Eric Dumazet @ 2010-12-19 15:43 UTC (permalink / raw)
To: Paweł Staszewski; +Cc: Linux Network Development list
In-Reply-To: <4D0DEDF9.7020102@itcare.pl>
Le dimanche 19 décembre 2010 à 12:35 +0100, Paweł Staszewski a écrit :
> Hi all
>
> I have panic with kernel 2.6.37-rc6-git2 when use iproute2
> mirred/redirect action
>
>
> host1 (kernel 2.6.36.2)
> netperf client -> eth3 (82598EB 10-Gigabit AT CX4) - directly connected
> to eth2 of host2
> ethtool -k eth3
> Offload parameters for eth3:
> rx-checksumming: on
> tx-checksumming: on
> scatter-gather: on
> tcp-segmentation-offload: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: on
> generic-receive-offload: on
> large-receive-offload: off
> rx-vlan-offload: off
> tx-vlan-offload: off
> ntuple-filters: off
> receive-hashing: off
>
> ethtool -i eth3
> driver: ixgbe
> version: 2.0.84-k2
> firmware-version: 1.12-2
> bus-info: 0000:03:00.1
>
>
> host2 (kernel-2.6.37-rc6-git2)
> netserver -> eth2 (82598EB 10-Gigabit AT CX4) - directly connected to
> eth3 of host1
>
> ethtool -k eth2
> Offload parameters for eth2:
> rx-checksumming: on
> tx-checksumming: on
> scatter-gather: on
> tcp-segmentation-offload: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: on
> generic-receive-offload: on
> large-receive-offload: off
> rx-vlan-offload: on
> tx-vlan-offload: on
> ntuple-filters: off
> receive-hashing: off
>
> ethtool -i eth2
> driver: ixgbe
> version: 2.0.84-k2
> firmware-version: 1.12-2
> bus-info: 0000:03:00.0
>
>
>
> Normally without ifb and redirect netperf show:
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.0.2
> (192.168.0.2) port 0 AF_INET
> Recv Send Send
> Socket Socket Message Elapsed
> Size Size Size Time Throughput
> bytes bytes bytes secs. 10^6bits/sec
>
> 87380 16384 16384 10.00 9042.14
>
>
> Steps to reproduce panic:
> ip link set dev ifb0 up
>
> tc qdisc add dev eth2 ingress
>
> tc filter add dev eth2 parent ffff: protocol ip prio 10 u32 \
> match ip src 0.0.0.0/0 flowid 1:1 \
> action mirred egress redirect dev ifb0
>
>
> After this when i start netperf on host1 I have panic (screenshot in
> attached image).
Unfortunately, we miss the start of panic messages. Could you try to get
them ?
^ permalink raw reply
* Re: [PATCHv4] fragment locally generated tunnel-mode IPSec6 packets as needed
From: David Stevens @ 2010-12-19 16:07 UTC (permalink / raw)
To: David Miller; +Cc: herbert, netdev
In-Reply-To: <20101218.143423.189688024.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote on 12/18/2010 02:34:23 PM:
> So is the TAHI test regression caused by v3 fixed here in v4?
Yes.
+-DLS
^ permalink raw reply
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Paweł Staszewski @ 2010-12-19 16:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Network Development list
In-Reply-To: <1292773433.18869.153.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 2506 bytes --]
W dniu 2010-12-19 16:43, Eric Dumazet pisze:
> Le dimanche 19 décembre 2010 à 12:35 +0100, Paweł Staszewski a écrit :
>> Hi all
>>
>> I have panic with kernel 2.6.37-rc6-git2 when use iproute2
>> mirred/redirect action
>>
>>
>> host1 (kernel 2.6.36.2)
>> netperf client -> eth3 (82598EB 10-Gigabit AT CX4) - directly connected
>> to eth2 of host2
>> ethtool -k eth3
>> Offload parameters for eth3:
>> rx-checksumming: on
>> tx-checksumming: on
>> scatter-gather: on
>> tcp-segmentation-offload: on
>> udp-fragmentation-offload: off
>> generic-segmentation-offload: on
>> generic-receive-offload: on
>> large-receive-offload: off
>> rx-vlan-offload: off
>> tx-vlan-offload: off
>> ntuple-filters: off
>> receive-hashing: off
>>
>> ethtool -i eth3
>> driver: ixgbe
>> version: 2.0.84-k2
>> firmware-version: 1.12-2
>> bus-info: 0000:03:00.1
>>
>>
>> host2 (kernel-2.6.37-rc6-git2)
>> netserver -> eth2 (82598EB 10-Gigabit AT CX4) - directly connected to
>> eth3 of host1
>>
>> ethtool -k eth2
>> Offload parameters for eth2:
>> rx-checksumming: on
>> tx-checksumming: on
>> scatter-gather: on
>> tcp-segmentation-offload: on
>> udp-fragmentation-offload: off
>> generic-segmentation-offload: on
>> generic-receive-offload: on
>> large-receive-offload: off
>> rx-vlan-offload: on
>> tx-vlan-offload: on
>> ntuple-filters: off
>> receive-hashing: off
>>
>> ethtool -i eth2
>> driver: ixgbe
>> version: 2.0.84-k2
>> firmware-version: 1.12-2
>> bus-info: 0000:03:00.0
>>
>>
>>
>> Normally without ifb and redirect netperf show:
>> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.0.2
>> (192.168.0.2) port 0 AF_INET
>> Recv Send Send
>> Socket Socket Message Elapsed
>> Size Size Size Time Throughput
>> bytes bytes bytes secs. 10^6bits/sec
>>
>> 87380 16384 16384 10.00 9042.14
>>
>>
>> Steps to reproduce panic:
>> ip link set dev ifb0 up
>>
>> tc qdisc add dev eth2 ingress
>>
>> tc filter add dev eth2 parent ffff: protocol ip prio 10 u32 \
>> match ip src 0.0.0.0/0 flowid 1:1 \
>> action mirred egress redirect dev ifb0
>>
>>
>> After this when i start netperf on host1 I have panic (screenshot in
>> attached image).
> Unfortunately, we miss the start of panic messages. Could you try to get
> them ?
>
In attached images
Regards
Pawel
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
[-- Attachment #2: kpanic-part1.JPG --]
[-- Type: image/jpeg, Size: 107573 bytes --]
[-- Attachment #3: kpanic-part2.JPG --]
[-- Type: image/jpeg, Size: 90615 bytes --]
^ permalink raw reply
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Changli Gao @ 2010-12-19 16:22 UTC (permalink / raw)
To: Paweł Staszewski; +Cc: Eric Dumazet, Linux Network Development list
In-Reply-To: <4D0E2E52.3090802@itcare.pl>
2010/12/20 Paweł Staszewski <pstaszewski@itcare.pl>:
> W dniu 2010-12-19 16:43, Eric Dumazet pisze:
>>
>> Unfortunately, we miss the start of panic messages. Could you try to get
>> them ?
>>
> In attached images
>
It seems the kernel panic at:
if (skb_shared(skb))
BUG();
in pskb_expand_head().
It maybe related to my patch:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=210d6de78c5d7c785fc532556cea340e517955e1
You can try to revert it and test again.
However, the bug is a misuse of pskb_expand_head().
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [PATCH] ueagle-atm: fix PHY signal initialization race
From: Dan Williams @ 2010-12-19 18:17 UTC (permalink / raw)
To: netdev; +Cc: Duncan Sands, linux-usb, Hicham HAOUARI
A race exists when initializing ueagle-atm devices where the generic atm
device may not yet be created before the driver attempts to initialize
it's PHY signal state, which checks whether the atm device has been
created or not. This often causes the sysfs 'carrier' attribute to be
'1' even though no signal has actually been found.
uea_probe
usbatm_usb_probe
driver->bind (uea_bind)
uea_boot
kthread_run(uea_kthread) uea_kthread
usbatm_atm_init uea_start_reset
atm_dev_register UPDATE_ATM_SIGNAL
UPDATE_ATM_SIGNAL checks whether the ATM device has been created and if
not, will not update the PHY signal state. Because of the race that
does not always happen in time, and the PHY signal state remains
ATM_PHY_SIG_FOUND even though no signal exists.
To fix the race, just create the kthread during initialization, and only
after initialization is complete, start the thread that reboots the
device and initializes PHY state.
[ 3030.490931] uea_probe: calling usbatm_usb_probe
[ 3030.490946] ueagle-atm 8-2:1.0: usbatm_usb_probe: trying driver ueagle-atm with vendor=1110, product=9031, ifnum 0
[ 3030.493691] uea_bind: setting usbatm
[ 3030.496932] usb 8-2: [ueagle-atm] using iso mode
[ 3030.497283] ueagle-atm 8-2:1.0: usbatm_usb_probe: using 3021 byte buffer for rx channel 0xffff880125953508
<kthread already started before usbatm_usb_probe() has returned>
[ 3030.497292] usb 8-2: [ueagle-atm] (re)booting started
<UPDATE_ATM_SIGNAL checks whether ATM device has been created yet before setting PHY state>
[ 3030.497298] uea_start_reset: atm dev (null)
<and since it hasn't been created yet PHY state is not set>
[ 3030.497306] ueagle-atm 8-2:1.0: usbatm_usb_probe: using 3392 byte buffer for tx channel 0xffff8801259535b8
[ 3030.497374] usbatm_usb_probe: about to init
[ 3030.497379] usbatm_usb_probe: calling usbatm_atm_init
<atm device finally gets created>
[ 3030.497384] usbatm_atm_init: creating atm device!
Signed-off-by: Dan Williams <dcbw@redhat.com>
---
diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 44447f5..99ac70e 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -2206,8 +2206,11 @@ static int uea_boot(struct uea_softc *sc)
goto err1;
}
- sc->kthread = kthread_run(uea_kthread, sc, "ueagle-atm");
- if (sc->kthread == ERR_PTR(-ENOMEM)) {
+ /* Create worker thread, but don't start it here. Start it after
+ * all usbatm generic initialization is done.
+ */
+ sc->kthread = kthread_create(uea_kthread, sc, "ueagle-atm");
+ if (IS_ERR(sc->kthread)) {
uea_err(INS_TO_USBDEV(sc), "failed to create thread\n");
goto err2;
}
@@ -2624,6 +2627,7 @@ static struct usbatm_driver uea_usbatm_driver = {
static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_device *usb = interface_to_usbdev(intf);
+ int ret;
uea_enters(usb);
uea_info(usb, "ADSL device founded vid (%#X) pid (%#X) Rev (%#X): %s\n",
@@ -2637,7 +2641,19 @@ static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (UEA_IS_PREFIRM(id))
return uea_load_firmware(usb, UEA_CHIP_VERSION(id));
- return usbatm_usb_probe(intf, id, &uea_usbatm_driver);
+ ret = usbatm_usb_probe(intf, id, &uea_usbatm_driver);
+ if (ret == 0) {
+ struct usbatm_data *usbatm = usb_get_intfdata(intf);
+ struct uea_softc *sc = usbatm->driver_data;
+
+ /* Ensure carrier is initialized to off as early as possible */
+ UPDATE_ATM_SIGNAL(ATM_PHY_SIG_LOST);
+
+ /* Only start the worker thread when all init is done */
+ wake_up_process(sc->kthread);
+ }
+
+ return ret;
}
static void uea_disconnect(struct usb_interface *intf)
^ permalink raw reply related
* [PATCH] batman-adv: Return hna count on local buffer fill
From: Sven Eckelmann @ 2010-12-19 19:28 UTC (permalink / raw)
To: davem; +Cc: netdev, Sven Eckelmann
hna_local_fill_buffer must return the number of added hna entries and
not the last checked hash bucket.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
This patch is for net-next-2.6
net/batman-adv/translation-table.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index a19e16c..a633b5a4 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -162,7 +162,7 @@ int hna_local_fill_buffer(struct bat_priv *bat_priv,
atomic_set(&bat_priv->hna_local_changed, 0);
spin_unlock_bh(&bat_priv->hna_lhash_lock);
- return i;
+ return count;
}
int hna_local_seq_print_text(struct seq_file *seq, void *offset)
--
1.7.2.3
^ permalink raw reply related
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Paweł Staszewski @ 2010-12-19 20:34 UTC (permalink / raw)
To: Changli Gao; +Cc: Eric Dumazet, Linux Network Development list
In-Reply-To: <AANLkTimMY3D_HssWb=Sw=b3tsxEKACk2QUzFzi-HaC-J@mail.gmail.com>
W dniu 2010-12-19 17:22, Changli Gao pisze:
> 2010/12/20 Paweł Staszewski<pstaszewski@itcare.pl>:
>> W dniu 2010-12-19 16:43, Eric Dumazet pisze:
>>> Unfortunately, we miss the start of panic messages. Could you try to get
>>> them ?
>>>
>> In attached images
>>
> It seems the kernel panic at:
>
> if (skb_shared(skb))
> BUG();
>
> in pskb_expand_head().
>
> It maybe related to my patch:
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=210d6de78c5d7c785fc532556cea340e517955e1
>
> You can try to revert it and test again.
>
> However, the bug is a misuse of pskb_expand_head().
>
patching file net/sched/act_mirred.c
Hunk #1 FAILED at 169.
Hunk #2 succeeded at 195 (offset 10 lines).
1 out of 2 hunks FAILED -- saving rejects to file net/sched/act_mirred.c.rej
***************
*** 169,181 ****
goto out;
}
- at = G_TC_AT(skb->tc_verd);
- skb2 = skb_act_clone(skb, GFP_ATOMIC, m->tcf_action);
if (skb2 == NULL)
goto out;
m->tcf_bstats.bytes += qdisc_pkt_len(skb2);
m->tcf_bstats.packets++;
if (!(at & AT_EGRESS)) {
if (m->tcfm_ok_push)
skb_push(skb2, skb2->dev->hard_header_len);
--- 169,181 ----
goto out;
}
+ skb2 = skb_act_clone(skb, GFP_ATOMIC);
if (skb2 == NULL)
goto out;
m->tcf_bstats.bytes += qdisc_pkt_len(skb2);
m->tcf_bstats.packets++;
+ at = G_TC_AT(skb->tc_verd);
if (!(at & AT_EGRESS)) {
if (m->tcfm_ok_push)
skb_push(skb2, skb2->dev->hard_header_len);
for sch_generic.h was ok.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: Add Nic partitioning mode (57712 devices)
From: Ben Hutchings @ 2010-12-19 21:21 UTC (permalink / raw)
To: Matt Domsch
Cc: Eilon Greenstein, Dimitris Michailidis, Dmitry Kravkov,
davem@davemloft.net, netdev@vger.kernel.org, narendra_k@dell.com,
jordan_hargrave@dell.com
In-Reply-To: <20101219055731.GD5854@auslistsprd01.us.dell.com>
On Sat, 2010-12-18 at 23:57 -0600, Matt Domsch wrote:
> On Fri, Dec 17, 2010 at 01:22:37PM +0000, Ben Hutchings wrote:
> > On Thu, 2010-12-16 at 20:45 -0600, Matt Domsch wrote:
> > > On Thu, Dec 09, 2010 at 04:49:25PM +0200, Eilon Greenstein wrote:
> > In the case of sfc, each port has a separate PCI function. We read this
> > register field to find out which port we're talking to, as
> > virtualisation can alter the function number. I don't know about the
> > others.
>
> For a single card then, this makes sense.
>
> pci<slot>#<port> where port = dev_id
>
> If I have 2 such cards on a PCI extender though, I think this breaks.
> Here, I'd see duplicate dev_id values, yes?
>
> Do you label the ports on your cards in any fashion? Do they have
> labels like port 0, port 1, port 2, ... ? Does it matter if we give
> names starting at 0, or starting at 1? latest biosdevname starts them
> at 1, or uses whatever value BIOS actually provides, which on systems
> I've tried, all start at 1.
[...]
Currently they aren't labelled, so far as I can aware.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC PATCH 02/12] net: Introduce new feature setting ops
From: Ben Hutchings @ 2010-12-19 21:22 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20101219004959.GA12005@rere.qmqm.pl>
On Sun, 2010-12-19 at 01:49 +0100, Michał Mirosław wrote:
> On Thu, Dec 16, 2010 at 11:13:06PM +0000, Ben Hutchings wrote:
> > On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
[...]
> > > +static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
> > > +{
> > > + struct ethtool_features cmd;
> > > + struct ethtool_set_features_block features[ETHTOOL_DEV_FEATURE_WORDS];
> > > +
> > > + if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
> > > + return -EFAULT;
> > > + useraddr += sizeof(cmd);
> > > +
> > > + if (cmd.count > ETHTOOL_DEV_FEATURE_WORDS)
> > > + cmd.count = ETHTOOL_DEV_FEATURE_WORDS;
> > So additional feature words will be silently ignored...
> > > + if (copy_from_user(features, useraddr, sizeof(*features) * cmd.count))
> > > + return -EFAULT;
> > > + memset(features + cmd.count, 0,
> > > + sizeof(features) - sizeof(*features) * cmd.count);
> > > +
> > > + features[0].valid &= dev->hw_features | NETIF_F_SOFT_FEATURES;
> > [...]
> >
> > ...as will any other unsupported features. This is not a good idea.
> > (However, remembering which features are wanted does seem like a good
> > idea.)
>
> That's intentional. Unsupported features can't be enabled anyway.
> hw_features is supposed to contain all features that the device can support
> and is able to enable/disable. This set should be constant and anything that
> is in the wanted_features set but is not supported because of other conditions
> will be stripped by ndo_fix_features() call.
>
> Other way would be to return EINVAL when bits not changeable are present in
> the valid mask. I don't want to do that, since then your example of changing
> a feature without GFEATURES first will not work.
That's right, it shouldn't work.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC PATCH] net_sched: sch_sfq: better struct layouts
From: Jarek Poplawski @ 2010-12-19 21:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Patrick McHardy, David Miller, netdev
In-Reply-To: <1292604766.2906.51.camel@edumazet-laptop>
On Fri, Dec 17, 2010 at 05:52:46PM +0100, Eric Dumazet wrote:
> Le jeudi 16 décembre 2010 ?? 14:08 +0100, Eric Dumazet a écrit :
...
> > struct sfq_slot {
> > struct sk_buff *first;
> > struct sk_buff *last;
> > u8 qlen;
> > sfq_index next; /* dequeue chain */
> > u16 hash;
> > short allot;
> > /* 16bit hole */
> > };
> >
> > This would save 768 bytes on x86_64 (and much more if LOCKDEP is used)
I think open coding sk_buff_head is a wrong idea. Otherwise, this
patch looks OK to me, only a few cosmetic suggestions below.
>
> Here is a preliminary patch, shrinking sizeof(struct sfq_sched_data)
> from 0x14f8 (or more if spinlocks are bigger) to 0x1180 bytes, and
> reduce text size as well.
>
> text data bss dec hex filename
> 4821 152 0 4973 136d old/net/sched/sch_sfq.o
> 4651 136 0 4787 12b3 new/net/sched/sch_sfq.o
>
>
> All data for a slot/flow is now grouped in a compact and cache friendly
> structure :
>
> struct sfq_slot {
> struct sk_buff *skblist_next;
> struct sk_buff *skblist_prev;
> sfq_index qlen; /* number of skbs in skblist */
> sfq_index next; /* next slot in sfq chain */
> unsigned short hash; /* hash value (index in ht[]) */
> short allot; /* credit for this slot */
> struct sfq_head anchor; /* anchor in dep[] chains */
> };
>
>
>
> net/sched/sch_sfq.c | 223 +++++++++++++++++++++++-------------------
> 1 file changed, 125 insertions(+), 98 deletions(-)
>
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 3cf478d..28968eb 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -69,25 +69,40 @@
> This implementation limits maximal queue length to 128;
> maximal mtu to 2^15-1; number of hash buckets to 1024.
> The only goal of this restrictions was that all data
> - fit into one 4K page :-). Struct sfq_sched_data is
> - organized in anti-cache manner: all the data for a bucket
> - are scattered over different locations. This is not good,
> - but it allowed me to put it into 4K.
> + fit into one 4K page on 32bit arches.
>
> It is easy to increase these values, but not in flight. */
>
> -#define SFQ_DEPTH 128
> +#define SFQ_DEPTH 128 /* max number of packets per slot (per flow) */
> +#define SFQ_SLOTS 128 /* max number of flows */
> +#define EMPTY_SLOT 255
SFQ_EMPTY_SLOT?
> #define SFQ_HASH_DIVISOR 1024
>
> -/* This type should contain at least SFQ_DEPTH*2 values */
> +/* This type should contain at least SFQ_DEPTH + SFQ_SLOTS values */
> typedef unsigned char sfq_index;
>
> +/*
> + * We dont use pointers to save space.
> + * Small indexes [0 ... SFQ_SLOTS - 1] are 'pointers' to slots[] array
> + * while following values [SFQ_SLOTS ... SFQ_SLOTS + SFQ_DEPTH - 1]
> + * are 'pointers' to dep[] array
> + */
> struct sfq_head
> {
> sfq_index next;
> sfq_index prev;
> };
>
> +struct sfq_slot {
> + struct sk_buff *skblist_next;
> + struct sk_buff *skblist_prev;
> + sfq_index qlen; /* number of skbs in skblist */
> + sfq_index next; /* next slot in sfq chain */
> + unsigned short hash; /* hash value (index in ht[]) */
> + short allot; /* credit for this slot */
> + struct sfq_head anchor; /* anchor in dep[] chains */
struct sfq_head dep?
> +};
> +
> struct sfq_sched_data
> {
> /* Parameters */
> @@ -99,17 +114,24 @@ struct sfq_sched_data
> struct tcf_proto *filter_list;
> struct timer_list perturb_timer;
> u32 perturbation;
> - sfq_index tail; /* Index of current slot in round */
> - sfq_index max_depth; /* Maximal depth */
> + sfq_index max_depth; /* depth of longest slot */
depth and/or length? (One dimension should be enough.)
>
> + struct sfq_slot *tail; /* current slot in round */
> sfq_index ht[SFQ_HASH_DIVISOR]; /* Hash table */
> - sfq_index next[SFQ_DEPTH]; /* Active slots link */
> - short allot[SFQ_DEPTH]; /* Current allotment per slot */
> - unsigned short hash[SFQ_DEPTH]; /* Hash value indexed by slots */
> - struct sk_buff_head qs[SFQ_DEPTH]; /* Slot queue */
> - struct sfq_head dep[SFQ_DEPTH*2]; /* Linked list of slots, indexed by depth */
> + struct sfq_slot slots[SFQ_SLOTS];
> + struct sfq_head dep[SFQ_DEPTH]; /* Linked list of slots, indexed by depth */
> };
>
> +/*
> + * sfq_head are either in a sfq_slot or in dep[] array
> + */
> +static inline struct sfq_head *get_head(struct sfq_sched_data *q, sfq_index val)
static inline struct sfq_head *sfq_dep_head()?
...
> @@ -304,31 +328,36 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> hash--;
>
> x = q->ht[hash];
> - if (x == SFQ_DEPTH) {
> - q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
> - q->hash[x] = hash;
> + slot = &q->slots[x];
> + if (x == EMPTY_SLOT) {
> + x = q->dep[0].next; /* get a free slot */
> + q->ht[hash] = x;
> + slot = &q->slots[x];
> + slot->hash = hash;
> + slot->skblist_next = slot->skblist_prev = (struct sk_buff *)slot;
> }
>
> - /* If selected queue has length q->limit, this means that
> - * all another queues are empty and that we do simple tail drop,
No reason to remove this line.
> + /* If selected queue has length q->limit, do simple tail drop,
> * i.e. drop _this_ packet.
> */
> - if (q->qs[x].qlen >= q->limit)
> + if (slot->qlen >= q->limit)
> return qdisc_drop(skb, sch);
>
> sch->qstats.backlog += qdisc_pkt_len(skb);
> - __skb_queue_tail(&q->qs[x], skb);
> + skb->prev = slot->skblist_prev;
> + skb->next = (struct sk_buff *)slot;
> + slot->skblist_prev->next = skb;
> + slot->skblist_prev = skb;
If you really have to do this, all these: __skb_queue_tail(),
__skb_dequeue(), skb_queue_head_init(), skb_peek() etc. used here
should stay as (local) inline functions to remain readability.
Jarek P.
^ permalink raw reply
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Jarek Poplawski @ 2010-12-19 22:15 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Changli Gao, Eric Dumazet, Linux Network Development list
In-Reply-To: <4D0E6C6D.10806@itcare.pl>
Paweł Staszewski wrote:
> W dniu 2010-12-19 17:22, Changli Gao pisze:
>> 2010/12/20 Paweł Staszewski<pstaszewski@itcare.pl>:
>>> W dniu 2010-12-19 16:43, Eric Dumazet pisze:
>>>> Unfortunately, we miss the start of panic messages. Could you try to
>>>> get
>>>> them ?
>>>>
>>> In attached images
>>>
>> It seems the kernel panic at:
>>
>> if (skb_shared(skb))
>> BUG();
>>
>> in pskb_expand_head().
>>
>> It maybe related to my patch:
>> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=210d6de78c5d7c785fc532556cea340e517955e1
>>
>>
>> You can try to revert it and test again.
>>
>> However, the bug is a misuse of pskb_expand_head().
>>
> patching file net/sched/act_mirred.c
> Hunk #1 FAILED at 169.
> Hunk #2 succeeded at 195 (offset 10 lines).
> 1 out of 2 hunks FAILED -- saving rejects to file
> net/sched/act_mirred.c.rej
>
> ***************
> *** 169,181 ****
> goto out;
> }
>
> - at = G_TC_AT(skb->tc_verd);
> - skb2 = skb_act_clone(skb, GFP_ATOMIC, m->tcf_action);
> if (skb2 == NULL)
> goto out;
>
> m->tcf_bstats.bytes += qdisc_pkt_len(skb2);
> m->tcf_bstats.packets++;
> if (!(at & AT_EGRESS)) {
> if (m->tcfm_ok_push)
> skb_push(skb2, skb2->dev->hard_header_len);
> --- 169,181 ----
> goto out;
> }
>
> + skb2 = skb_act_clone(skb, GFP_ATOMIC);
> if (skb2 == NULL)
> goto out;
>
> m->tcf_bstats.bytes += qdisc_pkt_len(skb2);
> m->tcf_bstats.packets++;
> + at = G_TC_AT(skb->tc_verd);
> if (!(at & AT_EGRESS)) {
> if (m->tcfm_ok_push)
> skb_push(skb2, skb2->dev->hard_header_len);
>
> for sch_generic.h was ok.
Should be enough to try after reverting this sch_generic.h change only.
Jarek P.
^ permalink raw reply
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Jarek Poplawski @ 2010-12-19 22:21 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Changli Gao, Eric Dumazet, Linux Network Development list
In-Reply-To: <4D0E8416.2030100@gmail.com>
Jarek Poplawski wrote:
> Paweł Staszewski wrote:
>> for sch_generic.h was ok.
>
> Should be enough to try after reverting this sch_generic.h change only.
Hmm... Sorry, I meant the change inside skb_act_clone(). I'll send a patch.
Jarek P.
^ permalink raw reply
* Re: Kernel panic eth2 mirred redirect to ifb0
From: Jarek Poplawski @ 2010-12-19 22:26 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Changli Gao, Eric Dumazet, Linux Network Development list
In-Reply-To: <4D0E857E.5060302@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 294 bytes --]
Jarek Poplawski wrote:
> Jarek Poplawski wrote:
>> Paweł Staszewski wrote:
>
>>> for sch_generic.h was ok.
>>
>> Should be enough to try after reverting this sch_generic.h change only.
>
> Hmm... Sorry, I meant the change inside skb_act_clone(). I'll send a patch.
Here it is.
Jarek P.
[-- Attachment #2: sch_generic.h.act_clone.1.diff --]
[-- Type: text/plain, Size: 644 bytes --]
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index ea1f8a8..8763ccc 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -608,13 +608,7 @@ static inline u32 qdisc_l2t(struct qdisc_rate_table* rtab, unsigned int pktlen)
static inline struct sk_buff *skb_act_clone(struct sk_buff *skb, gfp_t gfp_mask,
int action)
{
- struct sk_buff *n;
-
- if ((action == TC_ACT_STOLEN || action == TC_ACT_QUEUED) &&
- !skb_shared(skb))
- n = skb_get(skb);
- else
- n = skb_clone(skb, gfp_mask);
+ struct sk_buff *n = skb_clone(skb, gfp_mask);
if (n) {
n->tc_verd = SET_TC_VERD(n->tc_verd, 0);
^ permalink raw reply related
* Re: [RFC PATCH 02/12] net: Introduce new feature setting ops
From: Michał Mirosław @ 2010-12-19 23:43 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1292793759.2874.14.camel@localhost>
On Sun, Dec 19, 2010 at 09:22:39PM +0000, Ben Hutchings wrote:
> On Sun, 2010-12-19 at 01:49 +0100, Michał Mirosław wrote:
> > On Thu, Dec 16, 2010 at 11:13:06PM +0000, Ben Hutchings wrote:
> > > On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
> [...]
> > > > +static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
> > > > +{
> > > > + struct ethtool_features cmd;
> > > > + struct ethtool_set_features_block features[ETHTOOL_DEV_FEATURE_WORDS];
> > > > +
> > > > + if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
> > > > + return -EFAULT;
> > > > + useraddr += sizeof(cmd);
> > > > +
> > > > + if (cmd.count > ETHTOOL_DEV_FEATURE_WORDS)
> > > > + cmd.count = ETHTOOL_DEV_FEATURE_WORDS;
> > > So additional feature words will be silently ignored...
> > > > + if (copy_from_user(features, useraddr, sizeof(*features) * cmd.count))
> > > > + return -EFAULT;
> > > > + memset(features + cmd.count, 0,
> > > > + sizeof(features) - sizeof(*features) * cmd.count);
> > > > +
> > > > + features[0].valid &= dev->hw_features | NETIF_F_SOFT_FEATURES;
> > > [...]
> > >
> > > ...as will any other unsupported features. This is not a good idea.
> > > (However, remembering which features are wanted does seem like a good
> > > idea.)
> >
> > That's intentional. Unsupported features can't be enabled anyway.
> > hw_features is supposed to contain all features that the device can support
> > and is able to enable/disable. This set should be constant and anything that
> > is in the wanted_features set but is not supported because of other conditions
> > will be stripped by ndo_fix_features() call.
> >
> > Other way would be to return EINVAL when bits not changeable are present in
> > the valid mask. I don't want to do that, since then your example of changing
> > a feature without GFEATURES first will not work.
> That's right, it shouldn't work.
A user says "enable any TSO available". This means ethtool could issue
a request with .valid = NETIF_F_ALL_TSO, .requested = NETIF_F_ALL_TSO.
If the device supports only TSOv4 this should enable it and leave others
alone as whatever the user wants they can't be enabled.
This is 1-1 conversion of the semantics current ethtool ops have - set_tso
corresponds exactly to the request described above. This behaviour also
allows to execute a command like "enable as many as you can of ..." that
is usual goal of user enabling hardware offloads - to get best possible
performance.
Nevertheless, what problem is generated by ignoring unsupported bits here?
I can see the point of returning -EINVAL on bits that are not defined, though.
Is that a good direction?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCHv4] fragment locally generated tunnel-mode IPSec6 packets as needed
From: David Miller @ 2010-12-20 4:22 UTC (permalink / raw)
To: dlstevens; +Cc: herbert, netdev
In-Reply-To: <OFB6CDFBC7.E3781712-ON882577FE.0058793D-882577FE.00588A13@us.ibm.com>
From: David Stevens <dlstevens@us.ibm.com>
Date: Sun, 19 Dec 2010 08:07:06 -0800
> David Miller <davem@davemloft.net> wrote on 12/18/2010 02:34:23 PM:
>
>> So is the TAHI test regression caused by v3 fixed here in v4?
>
> Yes.
Great, applied, thanks.
^ permalink raw reply
* RE: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2010-12-20 4:29 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde
In-Reply-To: <4D0BD454.3060503-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Hi Wolfgang,
Thanks for the review.
Please see my replies in-line:
> here comes my first quick preview.
> On 12/15/2010 10:58 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/pdf/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.
> >
> > Changes since V1:
> > 1. Implemented C_CAN as a platform driver with means of providing the
> > platform details and register offsets which may vary for different
> SoCs
> > through platform data struct.
> > 2. Implemented NAPI.
> > 3. Removed memcpy calls globally.
> > 4. Implemented CAN_CTRLMODE_*
> > 5. Implemented and used priv->can.do_get_berr_counter.
> > 6. Implemented c_can registers as a struct instead of enum.
> > 7. Improved the TX path by implementing routines to get next Tx and
> echo msg
> > objects.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > ---
> > drivers/net/can/Kconfig | 7 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/c_can.c | 1217
> ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1225 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/net/can/c_can.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> > index 9d9e453..25d9d2e 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -41,6 +41,13 @@ config CAN_AT91
> > ---help---
> > This is a driver for the SoC CAN controller in Atmel's
> AT91SAM9263.
> >
> > +config CAN_C_CAN
> > + tristate "Bosch C_CAN controller"
> > + depends on CAN_DEV
> > + ---help---
> > + If you say yes to this option, support will be included for the
> > + Bosch C_CAN controller.
> > +
> > config CAN_TI_HECC
> > depends on CAN_DEV && ARCH_OMAP3
> > tristate "TI High End CAN Controller"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> > index 0057537..b6cbe74 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -12,6 +12,7 @@ obj-y += usb/
> > obj-$(CONFIG_CAN_SJA1000) += sja1000/
> > obj-$(CONFIG_CAN_MSCAN) += mscan/
> > obj-$(CONFIG_CAN_AT91) += at91_can.o
> > +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> > obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
> > obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
> > obj-$(CONFIG_CAN_BFIN) += bfin_can.o
> > diff --git a/drivers/net/can/c_can.c b/drivers/net/can/c_can.c
> > new file mode 100644
> > index 0000000..c281c17
> > --- /dev/null
> > +++ b/drivers/net/can/c_can.c
> > @@ -0,0 +1,1217 @@
> > +/*
> > + * 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/pdf/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/workqueue.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +
> > +#include <linux/can.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +
> > +#define DRV_NAME "c_can"
> > +
> > +/* control register */
> > +#define CONTROL_TEST (1 << 7)
> > +#define CONTROL_CCE (1 << 6)
> > +#define CONTROL_DISABLE_AR (1 << 5)
> > +#define CONTROL_ENABLE_AR (0 << 5)
> > +#define CONTROL_EIE (1 << 3)
> > +#define CONTROL_SIE (1 << 2)
> > +#define CONTROL_IE (1 << 1)
> > +#define CONTROL_INIT (1 << 0)
> > +
> > +/* test register */
> > +#define TEST_RX (1 << 7)
> > +#define TEST_TX1 (1 << 6)
> > +#define TEST_TX2 (1 << 5)
> > +#define TEST_LBACK (1 << 4)
> > +#define TEST_SILENT (1 << 3)
> > +#define TEST_BASIC (1 << 2)
> > +
> > +/* status register */
> > +#define STATUS_BOFF (1 << 7)
> > +#define STATUS_EWARN (1 << 6)
> > +#define STATUS_EPASS (1 << 5)
> > +#define STATUS_RXOK (1 << 4)
> > +#define STATUS_TXOK (1 << 3)
> > +#define STATUS_LEC_MASK 0x07
> > +#define LEC_STUFF_ERROR 1
> > +#define LEC_FORM_ERROR 2
> > +#define LEC_ACK_ERROR 3
> > +#define LEC_BIT1_ERROR 4
> > +#define LEC_BIT0_ERROR 5
> > +#define LEC_CRC_ERROR 6
>
> Could be an enum!?
Yes LEC error types can be defined as enum, but #define also
seems fine.
> > +/* error counter register */
> > +#define ERR_COUNTER_TEC_MASK 0xff
> > +#define ERR_COUNTER_TEC_SHIFT 0x0
> > +#define ERR_COUNTER_REC_SHIFT 8
> > +#define ERR_COUNTER_REC_MASK (0x7f << ERR_COUNTER_REC_SHIFT)
> > +#define ERR_COUNTER_RP_SHIFT 15
> > +#define ERR_COUNTER_RP_MASK (0x1 << ERR_COUNTER_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 (1 << 15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR (1 << 7)
> > +#define IF_COMM_MASK (1 << 6)
> > +#define IF_COMM_ARB (1 << 5)
> > +#define IF_COMM_CONTROL (1 << 4)
> > +#define IF_COMM_CLR_INT_PND (1 << 3)
> > +#define IF_COMM_TXRQST (1 << 2)
> > +#define IF_COMM_DATAA (1 << 1)
> > +#define IF_COMM_DATAB (1 << 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 (1 << 15)
> > +#define IF_ARB_MSGXTD (1 << 14)
> > +#define IF_ARB_TRANSMIT (1 << 13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT (1 << 15)
> > +#define IF_MCONT_MSGLST (1 << 14)
> > +#define IF_MCONT_INTPND (1 << 13)
> > +#define IF_MCONT_UMASK (1 << 12)
> > +#define IF_MCONT_TXIE (1 << 11)
> > +#define IF_MCONT_RXIE (1 << 10)
> > +#define IF_MCONT_RMTEN (1 << 9)
> > +#define IF_MCONT_TXRQST (1 << 8)
> > +#define IF_MCONT_EOB (1 << 7)
> > +
> > +/*
> > + * 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 31
> > +#define C_CAN_MSG_OBJ_RX_NUM 16
> > +#define C_CAN_MSG_OBJ_TX_NUM 16
> > +
> > +#define C_CAN_MSG_OBJ_RX_FIRST 0
> > +#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_NEXT_MSG_OBJ_MASK (C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS 0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT 0x8000
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > + u16 com_reg;
> > + u16 com_mask;
> > + u16 mask1;
> > + u16 mask2;
> > + u16 arb1;
> > + u16 arb2;
> > + u16 msg_cntrl;
> > + u16 data_a1;
> > + u16 data_a2;
> > + u16 data_b1;
> > + u16 data_b2;
> > + u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > + u16 control;
> > + u16 status;
> > + u16 error_counter;
> > + u16 btr;
> > + u16 ir;
> > + u16 test;
> > + u16 brp_ext;
> > + u16 _reserved1;
> > + struct c_can_if_regs ifreg[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 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
> > +};
>
> > +enum c_can_interrupt_mode {
> > + ENABLE_MODULE_INTERRUPT = 0,
> > + DISABLE_MODULE_INTERRUPT,
> > + ENABLE_ALL_INTERRUPTS,
> > + DISABLE_ALL_INTERRUPTS
> > +};
> > +
> > +/* 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 *reg_base;
> > + unsigned long irq_flags; /* for request_irq() */
> > + unsigned int tx_next;
> > + unsigned int tx_echo;
> > + struct clk *clk;
> > +};
> > +
> > +static struct can_bittiming_const c_can_bittiming_const = {
> > + .name = DRV_NAME,
> > + .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;
> > +}
> > +
> > +/* 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.
> > + */
>
> Nitpicking: please use here and in other places the recommended style
> for multi-line comments:
>
> /*
> * Comment ...
> */
>
Oops. Will be done in V3
> > +static u16 c_can_read_reg_aligned_to_16bit(void *reg)
> > +{
> > + return readw(reg);
> > +}
> > +
> > +static void c_can_write_reg_aligned_to_16bit(void *reg, u16 val)
> > +{
> > + writew(val, reg);
> > +}
>
> To profit from type checking, you should use "u16 __iomem *reg" instead
> of "void *reg". Also, I think iowrite16 is preferred nowadays.
>
> > +static u16 c_can_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> void *reg)
> > +{
> > + return readw(reg + (u32)reg - (u32)priv->reg_base);
> > +}
> > +
> > +static void c_can_write_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > + void *reg, u16 val)
> > +{
> > + writew(val, reg + (u32)reg - (u32)priv->reg_base);
> > +}
> > +
>
> This will not work properly on 64-bit systems. "(long)" should be used,
> at least. Any better ideas?
Hmm. I agree. Will incorporate this in V3.
> > +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 inline int c_can_configure_interrupts(struct c_can_priv
> *priv,
> > + enum c_can_interrupt_mode intr_mode)
> > +{
> > + unsigned int cntrl_save = priv->read_reg(priv,
> > + &priv->reg_base->control);
> > +
> > + switch (intr_mode) {
> > + case ENABLE_MODULE_INTERRUPT:
> > + cntrl_save |= CONTROL_IE;
> > + break;
> > + case DISABLE_MODULE_INTERRUPT:
> > + cntrl_save &= ~CONTROL_IE;
> > + break;
> > + case ENABLE_ALL_INTERRUPTS:
> > + cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> > + break;
> > + case DISABLE_ALL_INTERRUPTS:
> > + cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + priv->write_reg(priv, &priv->reg_base->control, cntrl_save);
> > +
> > + return 0;
> > +}
>
> Do you really need this function using a switch case. The first two
> cases are not used anywhere. I think
>
> void c_can_enable_all_interrupts(struct c_can_priv *priv, int
> enable);
>
> would be fine.
Right. V3 will reflect your suggestions.
> > +static inline int c_can_object_get(struct net_device *dev,
> > + int iface, int objno, int mask)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + int timeout = (6 / priv->can.clock.freq);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> > + IFX_WRITE_LOW_16BIT(mask));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> > + IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > + /* 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. The delay accounts for the same
> > + */
> > + udelay(timeout);
> > + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg))
> &
>
> I don't think you need the inner brackets.
Ok.
> > + IF_COMR_BUSY) {
> > + dev_info(dev->dev.parent, "timed out in object get\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static inline int c_can_object_put(struct net_device *dev,
> > + int iface, int objno, int mask)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + int timeout = (6 / priv->can.clock.freq);
>
> Hm, "timeout = 0" does not look resonable.
Let me see if I get your point here.
You mean use something like:
count = 6 /* non-zero count at start */
/* write message object no in IF COMM_REQ reg */
while (count) {
udelay(timeout);
count--;
}
/* read BUSY status from IF COM reg */
if (busy)
return -ETIMEDOUT;
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> > + (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> > + IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > + /* 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. The delay accounts for the same
> > + */
> > + udelay(timeout);
> > + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg))
> &
> > + IF_COMR_BUSY) {
> > + dev_info(dev->dev.parent, "timed out in object put\n");
>
> dev_err() seems more appropriate.
Ok.
> > + return -ETIMEDOUT;
> > + }
>
> Is the timeout really needed? If yes, re-trying various times would
> more
> more safe.
Yes timeout is needed as per specs. Please see the approach given above.
If you agree the same can be added in V3.
> > + return 0;
> > +}
> > +
> > +int c_can_write_msg_object(struct net_device *dev,
> > + int iface, struct can_frame *frame, int objno)
> > +{
> > + u16 flags = 0;
> > + unsigned int id;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + 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);
> > +
> > + if (!(frame->can_id & CAN_RTR_FLAG))
> > + flags |= IF_ARB_TRANSMIT;
> > +
> > + flags |= IF_ARB_MSGVAL;
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> > + IFX_WRITE_LOW_16BIT(id));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, flags |
> > + IFX_WRITE_HIGH_16BIT(id));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a1,
> > + (*(u16 *)(frame->data)));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a2,
> > + (*(u32 *)(frame->data)) >> 16);
> > +
> > + if (frame->can_dlc > 4) {
> > + priv->write_reg(priv, &priv->reg_base-
> >ifreg[iface].data_b1,
> > + (*(u16 *)(frame->data + 4)));
> > + priv->write_reg(priv, &priv->reg_base-
> >ifreg[iface].data_b2,
> > + (*(u32 *)(frame->data + 4)) >> 16);
> > + } else
> > + *(u32 *)(frame->data + 4) = 0;
>
> Is this code endianess safe?
Marc's suggestion to use data is an array (similar to
pch driver) seems better. Do you agree to the same?
> > +
> > + return frame->can_dlc;
> > +}
> > +
> > +static int c_can_read_msg_object(struct net_device *dev, int iface,
> int objno)
> > +{
> > + u16 flags;
> > + int ctrl;
> > + unsigned int val, data;
> > + 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;
> > + }
> > +
> > + val = c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> > + ~IF_COMM_TXRQST);
> > + if (val < 0)
> > + return val;
> > +
> > + ctrl = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].msg_cntrl);
> > + if (ctrl & IF_MCONT_MSGLST) {
> > + stats->rx_errors++;
> > + dev_info(dev->dev.parent, "msg lost in buffer %d\n",
> objno);
> > + }
>
> You should create an error message for that error as well.
Ok.
> > + frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> > + data = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].data_a1) |
> > + (priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].data_a2) <<
> > + 16);
> > + *(u32 *)(frame->data) = data;
> > + if (frame->can_dlc > 4) {
> > + data = priv->read_reg(priv,
> > + &priv->reg_base->ifreg[iface].data_b1) |
> > + (priv->read_reg(priv,
> > + &priv->reg_base->ifreg[iface].data_b2) <<
> > + 16);
> > + *(u32 *)(frame->data + 4) = data;
> > + } else
> > + *(u32 *)(frame->data + 4) = 0;
>
> Ditto.
Please see approach mentioned above.
> > + flags = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].arb2);
> > + val = priv->read_reg(priv, &priv->reg_base->ifreg[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;
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> ctrl &
> > + ~(IF_MCONT_MSGLST | IF_MCONT_INTPND |
> IF_MCONT_NEWDAT));
> > +
> > + val = c_can_object_put(dev, iface, objno, IF_COMM_CONTROL);
> > + if (val < 0)
> > + return val;
> > +
> > + netif_receive_skb(skb);
> > +
> > + stats->rx_packets++;
> > + stats->rx_bytes += frame->can_dlc;
> > +
> > + return 0;
>
> The return values are not handled anywhere!
Hmm. This is the tricky part. To be honest, a
lot of driver's don't handle all the return values.
This function is called from an isr / poll-event.
Do you think it's useful to handle the return values
there?
> > +}
> > +
> > +static int c_can_setup_receive_object(struct net_device *dev, int
> iface,
> > + int objno, unsigned int mask,
> > + unsigned int id, unsigned int mcont)
> > +{
> > + int ret;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask1,
> > + IFX_WRITE_LOW_16BIT(mask));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask2,
> > + IFX_WRITE_HIGH_16BIT(mask));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> > + IFX_WRITE_LOW_16BIT(id));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2,
> > + (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> mcont);
> > + ret = c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> > + ~IF_COMM_TXRQST);
> > + if (ret < 0)
> > + return ret;
>
> Ditto.
Please see comment above.
> > +
> > + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_inval_msg_object(struct net_device *dev, int iface,
> int objno)
> > +{
> > + int ret;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1, 0);
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, 0);
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> 0);
> > +
> > + ret = c_can_object_put(dev, iface, objno,
> > + IF_COMM_ARB | IF_COMM_CONTROL);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> > +
> > + return 0;
>
> Ditto.
Ditto.
> > +}
> > +
> > +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> > + struct net_device *dev)
> > +{
> > + u32 val;
> > + 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 */
> > + val = c_can_write_msg_object(dev, 0, frame, msg_obj_no);
> > +
> > + /* enable interrupt for this message object */
> > + priv->write_reg(priv, &priv->reg_base->ifreg[0].msg_cntrl,
> > + IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> > + (val & 0xf));
> > + val = c_can_object_put(dev, 0, msg_obj_no, IF_COMM_ALL);
> > + if (val < 0)
> > + return val;
> > +
> > + 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);
> > +
> > + 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;
> > +
> > + dev_dbg(dev->dev.parent,
> > + "brp = %d, brpe = %d, sjw = %d, seg1 = %d, seg2 =
> %d\n",
> > + brp, brpe, sjw, tseg1, tseg2);
> > + dev_dbg(dev->dev.parent, "setting BTR to %04x\n", reg_btr);
> > + dev_dbg(dev->dev.parent, "setting BRPE to %04x\n", reg_brpe);
>
> Like for the other drivers, could you please use one dev_info() here:
> dev_dbg(dev->dev.parent, "setting BTR=%04x BRPE=%04x\n", ...);
>
Ok. Will be done in V3
> > + ctrl_save = priv->read_reg(priv, &priv->reg_base->control);
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > + priv->write_reg(priv, &priv->reg_base->btr, reg_btr);
> > + priv->write_reg(priv, &priv->reg_base->brp_ext, reg_brpe);
> > + priv->write_reg(priv, &priv->reg_base->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.
> > + */
>
> Did you verify *in-order* transmisson and reception? You could use the
> canfdtest program from the can-utils.
I will check V3 for the same.
I also checked Marc's at91 driver and the
approach implemented there for in-order rx
object reception seems fine to me. If you and Marc agree I can
use the same here. Also I need to add credits for the same :)
> > +static int c_can_configure_msg_objects(struct net_device *dev)
> > +{
> > + int i;
> > +
> > + /* first invalidate all message objects */
> > + for (i = 0; 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 + 1 ; 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);
> > + return 0;
> > +}
> > +
> > +/*
> > + * Configure C_CAN chip:
> > + * - enable/disable auto-retransmission
> > + * - set operating mode
> > + * - configure message objects
> > + */
> > +static int 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->reg_base->control,
> > + CONTROL_DISABLE_AR);
> > + else
> > + /* enable automatic retransmission */
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + CONTROL_ENABLE_AR);
> > +
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > + /* loopback mode : useful for self-test function */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test, TEST_LBACK);
> > + } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> > + /* silent mode : bus-monitoring mode */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test, TEST_SILENT);
> > + } else if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> > + CAN_CTRLMODE_LOOPBACK)) {
>
> As I see it, this case is never entered.
You are right. But as we discussed during the review of V1,
as the c_can core supports this mode (loopback + listen-only)
we should support the same in the driver as well.
> > + /* loopback + silent mode : useful for hot self-test */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test,
> > + (TEST_LBACK | TEST_SILENT));
> > + } else
> > + /* normal mode*/
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + (CONTROL_EIE | CONTROL_SIE | CONTROL_IE));
> > +
> > + /* configure message objects */
> > + c_can_configure_msg_objects(dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_start(struct net_device *dev)
> > +{
> > + int err;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* enable status change, error and module interrupts */
> > + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +
> > + /* basic c_can configuration */
> > + err = c_can_chip_config(dev);
> > + if (err)
> > + return err;
> > +
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > + /* reset tx helper pointers */
> > + priv->tx_next = priv->tx_echo = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_stop(struct net_device *dev)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* disable all interrupts */
> > + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > + /* set the state as STOPPED */
> > + priv->can.state = CAN_STATE_STOPPED;
> > +
> > + return 0;
> > +}
> > +
> > +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);
> > + dev_info(dev->dev.parent,
> > + "c_can CAN_MODE_START requested\n");
>
> Please remove.
Ok.
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_get_state(const struct net_device *dev,
> > + enum can_state *state)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + *state = priv->can.state;
> > +
> > + return 0;
> > +}
>
> Please remove. This callback is only required if state changes cannot
> be
> mantained in the interrupt context.
Ok, V3 will handle this.
> > +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->reg_base-
> >error_counter);
> > + bec->rxerr = ((reg_err_counter & ERR_COUNTER_REC_MASK) >>
> > + ERR_COUNTER_REC_SHIFT);
> > + bec->txerr = (reg_err_counter & ERR_COUNTER_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->reg_base->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->reg_base->ifreg[0].msg_cntrl)
> > + & 0xF;
>
> Please use a #define for 0xf.
Ok.
> > + 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);
> > +}
> > +
> > +/*
> > + * c_can_do_rx_poll - read multiple CAN messages from message
> objects
> > + */
> > +static int c_can_do_rx_poll(struct net_device *dev, int quota)
> > +{
> > + u32 num_rx_pkts = 0;
> > + unsigned int msg_obj;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + u32 val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> > +
> > + while (val & RECEIVE_OBJECT_BITS) {
> > + for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> > + msg_obj <= C_CAN_MSG_OBJ_RX_LAST; msg_obj++) {
> > + if (val & (1 << msg_obj)) {
> > + c_can_read_msg_object(dev, 0, msg_obj);
> > + num_rx_pkts++;
> > + quota--;
>
> Where do you handle quota?
Sorry but I didn't get your meaning here.
Everytime the rx_poll function is called quota is
decremented and num of rx packets received is incremented.
Am I missing something here?
> > + }
> > + }
> > +
> > + val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> > + }
> > +
> > + return num_rx_pkts;
> > +}
> > +
> > +static int c_can_err(struct net_device *dev,
> > + enum c_can_bus_error_types error_type,
> > + int lec_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->reg_base-
> >error_counter);
> > + rx_err_passive = ((reg_err_counter & ERR_COUNTER_RP_MASK) >>
> > + ERR_COUNTER_RP_SHIFT);
> > +
> > + if (error_type & 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;
> > + if (bec.rxerr > 96)
> > + cf->data[1] = CAN_ERR_CRTL_RX_WARNING;
> > + if (bec.txerr > 96)
> > + cf->data[1] = CAN_ERR_CRTL_TX_WARNING;
> > + }
> > + if (error_type & 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;
> > + }
> > + if (error_type & 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_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > + can_bus_off(dev);
> > + }
> > +
> > + /* check for 'last error code' which tells us the
> > + * type of the last error to occur on the CAN bus
> > + */
> > + if (lec_type) {
> > + /* 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;
> > +
> > + if (lec_type & LEC_STUFF_ERROR) {
> > + dev_info(dev->dev.parent, "stuff error\n");
> > + cf->data[2] |= CAN_ERR_PROT_STUFF;
> > + }
> > + if (lec_type & LEC_FORM_ERROR) {
> > + dev_info(dev->dev.parent, "form error\n");
> > + cf->data[2] |= CAN_ERR_PROT_FORM;
> > + }
> > + if (lec_type & LEC_ACK_ERROR) {
> > + dev_info(dev->dev.parent, "ack error\n");
> > + cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> > + CAN_ERR_PROT_LOC_ACK_DEL);
> > + }
> > + if (lec_type & LEC_BIT1_ERROR) {
> > + dev_info(dev->dev.parent, "bit1 error\n");
> > + cf->data[2] |= CAN_ERR_PROT_BIT1;
> > + }
> > + if (lec_type & LEC_BIT0_ERROR) {
> > + dev_info(dev->dev.parent, "bit0 error\n");
> > + cf->data[2] |= CAN_ERR_PROT_BIT0;
> > + }
> > + if (lec_type & LEC_CRC_ERROR) {
> > + dev_info(dev->dev.parent, "CRC error\n");
>
> Please use dev_dbg() here and above
Ok.
> > + cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > + CAN_ERR_PROT_LOC_CRC_DEL);
> > + }
> > + }
>
> The lec should be handled by a switch statement. Also, please use
> dev_dbg in favor of dev_info.
But as I have seen on the board, there can be multiple lec bits
set at a time (e.g. shorting CAN TX and RX lines). In such cases the
multiple-if structure handles the same. Do you agree?
> > + 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);
> > + enum c_can_bus_error_types error_type = C_CAN_NO_ERROR;
> > +
> > + irqstatus = priv->read_reg(priv, &priv->reg_base->ir);
> > +
> > + /* status events have the highest priority */
> > + if (irqstatus == STATUS_INTERRUPT) {
> > + priv->current_status = priv->read_reg(priv,
> > + &priv->reg_base->status);
> > +
> > + /* handle Tx/Rx events */
> > + if (priv->current_status & STATUS_TXOK)
> > + priv->write_reg(priv, &priv->reg_base->status,
> > + (priv->current_status & ~STATUS_TXOK));
> > +
> > + if (priv->current_status & STATUS_RXOK)
> > + priv->write_reg(priv, &priv->reg_base->status,
> > + (priv->current_status & ~STATUS_RXOK));
> > +
> > + /* handle bus error events */
> > + if (priv->current_status & STATUS_EWARN) {
> > + dev_info(dev->dev.parent,
> > + "entered error warning state\n");
> > + error_type = C_CAN_ERROR_WARNING;
> > + }
> > + if ((priv->current_status & STATUS_EPASS) &&
> > + (!(priv->last_status & STATUS_EPASS))) {
> > + dev_info(dev->dev.parent,
> > + "entered error passive state\n");
> > + error_type = C_CAN_ERROR_PASSIVE;
> > + }
> > + if ((priv->current_status & STATUS_BOFF) &&
> > + (!(priv->last_status & STATUS_BOFF))) {
> > + dev_info(dev->dev.parent,
> > + "entered bus off state\n");
> > + error_type = C_CAN_BUS_OFF;
> > + }
> > + if (priv->current_status & STATUS_LEC_MASK)
> > + lec_type = (priv->current_status & STATUS_LEC_MASK);
> > +
> > + /* handle bus recovery events */
> > + if ((!(priv->current_status & STATUS_EPASS)) &&
> > + (priv->last_status & STATUS_EPASS)) {
> > + dev_info(dev->dev.parent,
> > + "left error passive state\n");
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + }
> > + if ((!(priv->current_status & STATUS_BOFF)) &&
> > + (priv->last_status & STATUS_BOFF)) {
> > + dev_info(dev->dev.parent,
> > + "left bus off state\n");
>
> Please use dev_dbg here and above.
Ok.
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + }
> > +
> > + priv->last_status = priv->current_status;
> > +
> > + /* handle error on the bus */
> > + if (error_type != C_CAN_NO_ERROR)
> > + work_done += c_can_err(dev, error_type, 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));
> > + quota--;
>
> Why do you decrement quota here?
Oops. Will be corrected in V3
> > + } 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);
> > + }
> > +
> > + if (work_done < quota) {
> > + napi_complete(napi);
> > + /* enable all IRQs */
> > + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > + }
> > +
> > + return work_done;
> > +}
> > +
> > +static irqreturn_t c_can_isr(int irq, void *dev_id)
> > +{
> > + struct net_device *dev = (struct net_device *)dev_id;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* disable all interrupts and schedule the NAPI */
> > + c_can_configure_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) {
> > + dev_err(dev->dev.parent, "failed to open can device\n");
> > + return err;
> > + }
> > +
> > + /* register interrupt handler */
> > + err = request_irq(dev->irq, &c_can_isr, priv->irq_flags, dev-
> >name,
> > + (void *)dev);
>
> I don't think you need the (void *) cast.
Ok.
> > + if (err < 0) {
> > + dev_err(dev->dev.parent, "failed to attach interrupt\n");
> > + goto exit_irq_fail;
> > + }
> > +
> > + /* start the c_can controller */
> > + err = c_can_start(dev);
> > + if (err)
> > + goto exit_start_fail;
> > + napi_enable(&priv->napi);
> > +
> > + netif_start_queue(dev);
> > +
> > + return 0;
> > +
> > +exit_start_fail:
> > + free_irq(dev->irq, dev);
> > +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;
> > +}
> > +
> > +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,
> > +};
> > +
> > +static int c_can_probe(struct platform_device *pdev)
>
> Please use __devinit ...
Ok.
> > +{
> > + 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),
> DRV_NAME)) {
> > + 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_candev(sizeof(struct c_can_priv),
> C_CAN_MSG_OBJ_TX_NUM);
> > + if (!dev) {
> > + ret = -ENOMEM;
> > + goto exit_iounmap;
> > + }
> > +
> > + priv = netdev_priv(dev);
> > +
> > + priv->irq_flags = irq->flags;
> > + priv->reg_base = addr;
> > + priv->can.clock.freq = clk_get_rate(clk);
> > + priv->clk = clk;
> > +
> > + switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> > + case IORESOURCE_MEM_32BIT:
> > + priv->read_reg = c_can_read_reg_aligned_to_32bit;
> > + priv->write_reg = c_can_write_reg_aligned_to_32bit;
> > + break;
> > + case IORESOURCE_MEM_16BIT:
> > + default:
> > + priv->read_reg = c_can_read_reg_aligned_to_16bit;
> > + priv->write_reg = c_can_write_reg_aligned_to_16bit;
> > + break;
> > + }
> > +
> > + priv->dev = dev;
> > + priv->can.bittiming_const = &c_can_bittiming_const;
> > + priv->can.do_set_bittiming = c_can_set_bittiming;
> > + priv->can.do_get_state = c_can_get_state;
> > + 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;
>
> Where is CAN_CTRLMODE_BERR_REPORTING implemented? Note that it has
> nothing to do with do_get_berr_counter. Please check the SJA1000
> driver:
>
> http://lxr.linux.no/#linux+v2.6.36/drivers/net/can/sja1000/sja1000.c#L1
> 46
>
> Bus error handling can be requested by the user via netlink interface.
>
> # ip link set canX type can ... berr-reporting on
>
> The driver then usually enables the bus error interrupts. I just
> realize
> that Documentation/networking/can.txt is not up-to-date. I will provide
> a patch a.s.a.p.
Yes, I have seen the sja1000 implementation before preparing V2.
But unfortunately the c_can core does not also only the bus-error-reporting
to be masked/unmasked. There are three interrupt masks available in the
Control register:
a) Error Interrupt Enable
If Enabled - A change in the bits BOff or EWarn in the Status Register will
generate an interrupt.
b) Status Change Interrupt Enable
If Enabled - An interrupt will be generated when a message transfer is
successfully completed or a CAN bus error is detected.
c) Module Interrupt Enable
If Enabled - Interrupts will set IRQ_B to LOW.
> > + netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> > +
> > + dev->irq = irq->start;
> > + dev->flags |= IFF_ECHO; /* we support local echo */
> > + dev->netdev_ops = &c_can_netdev_ops;
> > + platform_set_drvdata(pdev, dev);
> > + SET_NETDEV_DEV(dev, &pdev->dev);
> > +
> > + ret = register_candev(dev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> > + DRV_NAME, ret);
> > + goto exit_free_device;
> > + }
> > +
> > + dev_info(&pdev->dev, "%s device registered (reg_base=%p,
> irq=%d)\n",
> > + DRV_NAME, priv->reg_base, dev->irq);
> > + return 0;
> > +
> > +exit_free_device:
> > + platform_set_drvdata(pdev, NULL);
> > + free_candev(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 c_can_remove(struct platform_device *pdev)
>
> ... and __devexit.
Ok.
> > +{
> > + struct net_device *dev = platform_get_drvdata(pdev);
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + struct resource *mem;
> > +
> > + /* disable all interrupts */
> > + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > + unregister_candev(dev);
> > + platform_set_drvdata(pdev, NULL);
> > +
> > + free_candev(dev);
> > + iounmap(priv->reg_base);
> > +
> > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + release_mem_region(mem->start, resource_size(mem));
> > +
> > + clk_put(priv->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver c_can_driver = {
> > + .driver = {
> > + .name = DRV_NAME,
> > + .owner = THIS_MODULE,
> > + },
> > + .probe = c_can_probe,
> > + .remove = c_can_remove,
>
> Please use __devexit_p.
Ok.
> > +};
>
> No "=" alignment please.
Ok.
> > +static int __init c_can_init(void)
> > +{
> > + return platform_driver_register(&c_can_driver);
> > +}
> > +module_init(c_can_init);
> > +
> > +static void __exit c_can_exit(void)
> > +{
> > + platform_driver_unregister(&c_can_driver);
> > +}
> > +module_exit(c_can_exit);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("CAN bus driver for Bosch C_CAN controller");
>
> You could also use the new netdev_dbg and friends instead of
> dev_dbg(dev->dev.parent, ...).
Ok.
> Thanks for you contribution.
>
> Wolfgang.
Regards,
Bhupesh
^ permalink raw reply
* RE: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2010-12-20 4:37 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Wolfgang Grandegger
In-Reply-To: <4D0BD744.5030609-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Marc,
Thanks for the review.
Please see my comments in-line:
> No time to do a real review, some comments and lots of nitpicking
> inline....
>
> regards, Marc
>
> >
> > 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.
> >
> > Changes since V1:
> > 1. Implemented C_CAN as a platform driver with means of providing the
> > platform details and register offsets which may vary for different
> SoCs
> > through platform data struct.
> > 2. Implemented NAPI.
> > 3. Removed memcpy calls globally.
> > 4. Implemented CAN_CTRLMODE_*
> > 5. Implemented and used priv->can.do_get_berr_counter.
> > 6. Implemented c_can registers as a struct instead of enum.
> > 7. Improved the TX path by implementing routines to get next Tx and
> echo msg
> > objects.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > ---
> > drivers/net/can/Kconfig | 7 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/c_can.c | 1217
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1225 insertions(+), 0 deletions(-) create mode
> > 100644 drivers/net/can/c_can.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > 9d9e453..25d9d2e 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -41,6 +41,13 @@ config CAN_AT91
> > ---help---
> > This is a driver for the SoC CAN controller in Atmel's
> AT91SAM9263.
> >
> > +config CAN_C_CAN
> > + tristate "Bosch C_CAN controller"
> > + depends on CAN_DEV
> > + ---help---
> > + If you say yes to this option, support will be included for the
> > + Bosch C_CAN controller.
> > +
> > config CAN_TI_HECC
> > depends on CAN_DEV && ARCH_OMAP3
> > tristate "TI High End CAN Controller"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index
> > 0057537..b6cbe74 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -12,6 +12,7 @@ obj-y += usb/
> > obj-$(CONFIG_CAN_SJA1000) += sja1000/
> > obj-$(CONFIG_CAN_MSCAN) += mscan/
> > obj-$(CONFIG_CAN_AT91) += at91_can.o
> > +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> > obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
> > obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
> > obj-$(CONFIG_CAN_BFIN) += bfin_can.o
> > diff --git a/drivers/net/can/c_can.c b/drivers/net/can/c_can.c new
> > file mode 100644 index 0000000..c281c17
> > --- /dev/null
> > +++ b/drivers/net/can/c_can.c
> > @@ -0,0 +1,1217 @@
> > +/*
> > + * 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>
> > + *
>
> I recognize some stuff from the at91_can driver, too :)
Yes. And if you and Wolfgang agree to using the at91 approach
for in-order rx object reception here, I will be compelled to
add credits here :)
> > + * 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/pdf/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/workqueue.h>
> > +#include <linux/io.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +
> > +#include <linux/can.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +
> > +#define DRV_NAME "c_can"
>
> You can use KBUILD_MODNAME, no need to define DRV_NAME.
Ok.
> > +
> > +/* control register */
> > +#define CONTROL_TEST (1 << 7)
> > +#define CONTROL_CCE (1 << 6)
> > +#define CONTROL_DISABLE_AR (1 << 5)
> > +#define CONTROL_ENABLE_AR (0 << 5)
> > +#define CONTROL_EIE (1 << 3)
> > +#define CONTROL_SIE (1 << 2)
> > +#define CONTROL_IE (1 << 1)
> > +#define CONTROL_INIT (1 << 0)
> > +
> > +/* test register */
> > +#define TEST_RX (1 << 7)
> > +#define TEST_TX1 (1 << 6)
> > +#define TEST_TX2 (1 << 5)
> > +#define TEST_LBACK (1 << 4)
> > +#define TEST_SILENT (1 << 3)
> > +#define TEST_BASIC (1 << 2)
>
> You can use BIT(n) instead of (1 << n).
Ok.
> > +
> > +/* status register */
> > +#define STATUS_BOFF (1 << 7)
> > +#define STATUS_EWARN (1 << 6)
> > +#define STATUS_EPASS (1 << 5)
> > +#define STATUS_RXOK (1 << 4)
> > +#define STATUS_TXOK (1 << 3)
> > +#define STATUS_LEC_MASK 0x07
> > +#define LEC_STUFF_ERROR 1
> > +#define LEC_FORM_ERROR 2
> > +#define LEC_ACK_ERROR 3
> > +#define LEC_BIT1_ERROR 4
> > +#define LEC_BIT0_ERROR 5
> > +#define LEC_CRC_ERROR 6
> > +
> > +/* error counter register */
> > +#define ERR_COUNTER_TEC_MASK 0xff
> > +#define ERR_COUNTER_TEC_SHIFT 0x0
>
> nitpick, I'd just use a pure decimal 0 :)
Hmm. Ok :)
> > +#define ERR_COUNTER_REC_SHIFT 8
> > +#define ERR_COUNTER_REC_MASK (0x7f << ERR_COUNTER_REC_SHIFT)
> > +#define ERR_COUNTER_RP_SHIFT 15
> > +#define ERR_COUNTER_RP_MASK (0x1 << ERR_COUNTER_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 (1 << 15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR (1 << 7)
> > +#define IF_COMM_MASK (1 << 6)
> > +#define IF_COMM_ARB (1 << 5)
> > +#define IF_COMM_CONTROL (1 << 4)
> > +#define IF_COMM_CLR_INT_PND (1 << 3)
> > +#define IF_COMM_TXRQST (1 << 2)
> > +#define IF_COMM_DATAA (1 << 1)
> > +#define IF_COMM_DATAB (1 << 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 (1 << 15)
> > +#define IF_ARB_MSGXTD (1 << 14)
> > +#define IF_ARB_TRANSMIT (1 << 13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT (1 << 15)
> > +#define IF_MCONT_MSGLST (1 << 14)
> > +#define IF_MCONT_INTPND (1 << 13)
> > +#define IF_MCONT_UMASK (1 << 12)
> > +#define IF_MCONT_TXIE (1 << 11)
> > +#define IF_MCONT_RXIE (1 << 10)
> > +#define IF_MCONT_RMTEN (1 << 9)
> > +#define IF_MCONT_TXRQST (1 << 8)
> > +#define IF_MCONT_EOB (1 << 7)
> > +
> > +/*
> > + * 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 31
> > +#define C_CAN_MSG_OBJ_RX_NUM 16
> > +#define C_CAN_MSG_OBJ_TX_NUM 16
> > +
> > +#define C_CAN_MSG_OBJ_RX_FIRST 0
> > +#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_NEXT_MSG_OBJ_MASK (C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS 0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT 0x8000
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > + u16 com_reg;
> > + u16 com_mask;
> > + u16 mask1;
> > + u16 mask2;
> > + u16 arb1;
> > + u16 arb2;
> > + u16 msg_cntrl;
> > + u16 data_a1;
> > + u16 data_a2;
> > + u16 data_b1;
> > + u16 data_b2;
>
> The later code _mighy_ be easier to read if you define data as an array
> of u16, but let's see...
>
> > + u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > + u16 control;
> > + u16 status;
> > + u16 error_counter;
> > + u16 btr;
> > + u16 ir;
> > + u16 test;
> > + u16 brp_ext;
> > + u16 _reserved1;
> > + struct c_can_if_regs ifreg[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 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
> ^
> please add a ","
Oops. Will be done in V3.
> > +};
> > +
> > +enum c_can_interrupt_mode {
> > + ENABLE_MODULE_INTERRUPT = 0,
> > + DISABLE_MODULE_INTERRUPT,
> > + ENABLE_ALL_INTERRUPTS,
> > + DISABLE_ALL_INTERRUPTS
> same here
Ditto.
> > +};
> > +
> > +/* 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 *reg_base;
> > + unsigned long irq_flags; /* for request_irq() */
> > + unsigned int tx_next;
> > + unsigned int tx_echo;
> > + struct clk *clk;
> > +};
> > +
> > +static struct can_bittiming_const c_can_bittiming_const = {
> > + .name = DRV_NAME,
>
> use KBUILD_MODNAME here
OK, will be done in V3.
> > + .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;
> > +}
> > +
> > +/* 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.
> > + */
>
> /*
> * this is the preferred multi-line comment style,
> * please adjust
> */
Ok.
> > +static u16 c_can_read_reg_aligned_to_16bit(void *reg) {
> > + return readw(reg);
> > +}
> > +
> > +static void c_can_write_reg_aligned_to_16bit(void *reg, u16 val) {
> > + writew(val, reg);
> > +}
> > +
> > +static u16 c_can_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> > +void *reg) {
> > + return readw(reg + (u32)reg - (u32)priv->reg_base);
>
> as Wolfgang said not 64 bit safe.....what about casting the reg_base to
> void __iomem *?
As Wolfgang mentioned do you agree to use long here?
> > +}
> > +
> > +static void c_can_write_reg_aligned_to_32bit(struct c_can_priv
> *priv,
> > + void *reg, u16 val)
> > +{
> > + writew(val, reg + (u32)reg - (u32)priv->reg_base); }
> > +
> > +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 inline int c_can_configure_interrupts(struct c_can_priv
> *priv,
> > + enum c_can_interrupt_mode intr_mode) {
> > + unsigned int cntrl_save = priv->read_reg(priv,
> > + &priv->reg_base->control);
> > +
> > + switch (intr_mode) {
> > + case ENABLE_MODULE_INTERRUPT:
> > + cntrl_save |= CONTROL_IE;
> > + break;
> > + case DISABLE_MODULE_INTERRUPT:
> > + cntrl_save &= ~CONTROL_IE;
> > + break;
> > + case ENABLE_ALL_INTERRUPTS:
> > + cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> > + break;
> > + case DISABLE_ALL_INTERRUPTS:
> > + cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + priv->write_reg(priv, &priv->reg_base->control, cntrl_save);
> > +
> > + return 0;
> > +}
> > +
> > +static inline int c_can_object_get(struct net_device *dev,
> > + int iface, int objno, int mask)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + int timeout = (6 / priv->can.clock.freq);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> > + IFX_WRITE_LOW_16BIT(mask));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> > + IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > + /* 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. The delay accounts for the same
> > + */
> > + udelay(timeout);
> > + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg))
> &
> > + IF_COMR_BUSY) {
> > + dev_info(dev->dev.parent, "timed out in object get\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static inline int c_can_object_put(struct net_device *dev,
> > + int iface, int objno, int mask)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + int timeout = (6 / priv->can.clock.freq);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> > + (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> > + IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > + /* 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. The delay accounts for the same
> > + */
> > + udelay(timeout);
> > + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg))
> &
> > + IF_COMR_BUSY) {
> > + dev_info(dev->dev.parent, "timed out in object put\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int c_can_write_msg_object(struct net_device *dev,
> > + int iface, struct can_frame *frame, int objno) {
> > + u16 flags = 0;
> > + unsigned int id;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + 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);
> > +
> > + if (!(frame->can_id & CAN_RTR_FLAG))
> > + flags |= IF_ARB_TRANSMIT;
> > +
> > + flags |= IF_ARB_MSGVAL;
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> > + IFX_WRITE_LOW_16BIT(id));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, flags |
> > + IFX_WRITE_HIGH_16BIT(id));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a1,
> > + (*(u16 *)(frame->data)));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a2,
> > + (*(u32 *)(frame->data)) >> 16);
> > +
> > + if (frame->can_dlc > 4) {
> > + priv->write_reg(priv, &priv->reg_base-
> >ifreg[iface].data_b1,
> > + (*(u16 *)(frame->data + 4)));
> > + priv->write_reg(priv, &priv->reg_base-
> >ifreg[iface].data_b2,
> > + (*(u32 *)(frame->data + 4)) >> 16);
> > + } else
> > + *(u32 *)(frame->data + 4) = 0;
>
> look at the pch can driver, it uses an array for ifreg->data and is
> endianess safe.
I agree. Will use the same in V3.
> > +
> > + return frame->can_dlc;
> > +}
> > +
> > +static int c_can_read_msg_object(struct net_device *dev, int iface,
> > +int objno) {
> > + u16 flags;
> > + int ctrl;
> > + unsigned int val, data;
> > + 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;
> > + }
> > +
> > + val = c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> > + ~IF_COMM_TXRQST);
> > + if (val < 0)
> > + return val;
> > +
> > + ctrl = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].msg_cntrl);
> > + if (ctrl & IF_MCONT_MSGLST) {
> > + stats->rx_errors++;
> > + dev_info(dev->dev.parent, "msg lost in buffer %d\n",
> objno);
> > + }
> > +
> > + frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> > + data = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].data_a1) |
> > + (priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].data_a2) <<
> > + 16);
> > + *(u32 *)(frame->data) = data;
> > + if (frame->can_dlc > 4) {
> > + data = priv->read_reg(priv,
> > + &priv->reg_base->ifreg[iface].data_b1) |
> > + (priv->read_reg(priv,
> > + &priv->reg_base->ifreg[iface].data_b2) <<
> > + 16);
> > + *(u32 *)(frame->data + 4) = data;
> > + } else
> > + *(u32 *)(frame->data + 4) = 0;
>
> dito
Ditto.
> > +
> > + flags = priv->read_reg(priv, &priv->reg_base-
> >ifreg[iface].arb2);
> > + val = priv->read_reg(priv, &priv->reg_base->ifreg[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;
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> ctrl &
> > + ~(IF_MCONT_MSGLST | IF_MCONT_INTPND |
> IF_MCONT_NEWDAT));
> > +
> > + val = c_can_object_put(dev, iface, objno, IF_COMM_CONTROL);
> > + if (val < 0)
> > + return val;
> > +
> > + netif_receive_skb(skb);
> > +
> > + stats->rx_packets++;
> > + stats->rx_bytes += frame->can_dlc;
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_setup_receive_object(struct net_device *dev, int
> iface,
> > + int objno, unsigned int mask,
> > + unsigned int id, unsigned int mcont) {
> > + int ret;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask1,
> > + IFX_WRITE_LOW_16BIT(mask));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask2,
> > + IFX_WRITE_HIGH_16BIT(mask));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> > + IFX_WRITE_LOW_16BIT(id));
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2,
> > + (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> mcont);
> > + ret = c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> > + ~IF_COMM_TXRQST);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_inval_msg_object(struct net_device *dev, int iface,
> > +int objno) {
> > + int ret;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1, 0);
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, 0);
> > + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl,
> 0);
> > +
> > + ret = c_can_object_put(dev, iface, objno,
> > + IF_COMM_ARB | IF_COMM_CONTROL);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> > +
> > + return 0;
> > +}
> > +
> > +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> > + struct net_device *dev)
> > +{
> > + u32 val;
> > + 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 */
> > + val = c_can_write_msg_object(dev, 0, frame, msg_obj_no);
> > +
> > + /* enable interrupt for this message object */
> > + priv->write_reg(priv, &priv->reg_base->ifreg[0].msg_cntrl,
> > + IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> > + (val & 0xf));
> > + val = c_can_object_put(dev, 0, msg_obj_no, IF_COMM_ALL);
> > + if (val < 0)
> > + return val;
> > +
> > + 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);
> > +
> > + 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;
> > +
> > + dev_dbg(dev->dev.parent,
> > + "brp = %d, brpe = %d, sjw = %d, seg1 = %d, seg2 =
> %d\n",
> > + brp, brpe, sjw, tseg1, tseg2);
> > + dev_dbg(dev->dev.parent, "setting BTR to %04x\n", reg_btr);
> > + dev_dbg(dev->dev.parent, "setting BRPE to %04x\n", reg_brpe);
> > +
> > + ctrl_save = priv->read_reg(priv, &priv->reg_base->control);
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > + priv->write_reg(priv, &priv->reg_base->btr, reg_btr);
> > + priv->write_reg(priv, &priv->reg_base->brp_ext, reg_brpe);
> > + priv->write_reg(priv, &priv->reg_base->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 int c_can_configure_msg_objects(struct net_device *dev) {
> > + int i;
> > +
> > + /* first invalidate all message objects */
> > + for (i = 0; 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 + 1 ; 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);
> > + return 0;
> > +}
> > +
> > +/*
> > + * Configure C_CAN chip:
> > + * - enable/disable auto-retransmission
> > + * - set operating mode
> > + * - configure message objects
> > + */
> > +static int 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->reg_base->control,
> > + CONTROL_DISABLE_AR);
> > + else
> > + /* enable automatic retransmission */
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + CONTROL_ENABLE_AR);
> > +
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > + /* loopback mode : useful for self-test function */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test, TEST_LBACK);
> > + } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> > + /* silent mode : bus-monitoring mode */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test, TEST_SILENT);
> > + } else if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> > + CAN_CTRLMODE_LOOPBACK)) {
> > + /* loopback + silent mode : useful for hot self-test */
> > + priv->write_reg(priv, &priv->reg_base->control,
> (CONTROL_EIE |
> > + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> > + priv->write_reg(priv, &priv->reg_base->test,
> > + (TEST_LBACK | TEST_SILENT));
> > + } else
> > + /* normal mode*/
> > + priv->write_reg(priv, &priv->reg_base->control,
> > + (CONTROL_EIE | CONTROL_SIE | CONTROL_IE));
> > +
> > + /* configure message objects */
> > + c_can_configure_msg_objects(dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_start(struct net_device *dev) {
> > + int err;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* enable status change, error and module interrupts */
> > + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +
> > + /* basic c_can configuration */
> > + err = c_can_chip_config(dev);
> > + if (err)
> > + return err;
> > +
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > + /* reset tx helper pointers */
> > + priv->tx_next = priv->tx_echo = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_stop(struct net_device *dev) {
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* disable all interrupts */
> > + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > + /* set the state as STOPPED */
> > + priv->can.state = CAN_STATE_STOPPED;
> > +
> > + return 0;
> > +}
> > +
> > +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);
> > + dev_info(dev->dev.parent,
> > + "c_can CAN_MODE_START requested\n");
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int c_can_get_state(const struct net_device *dev,
> > + enum can_state *state)
> > +{
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + *state = priv->can.state;
> > +
> > + 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->reg_base-
> >error_counter);
> > + bec->rxerr = ((reg_err_counter & ERR_COUNTER_REC_MASK) >>
> > + ERR_COUNTER_REC_SHIFT);
> > + bec->txerr = (reg_err_counter & ERR_COUNTER_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->reg_base->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->reg_base->ifreg[0].msg_cntrl)
> > + & 0xF;
> > + 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);
> > +}
> > +
> > +/*
> > + * c_can_do_rx_poll - read multiple CAN messages from message
> objects
> > +*/ static int c_can_do_rx_poll(struct net_device *dev, int quota) {
> > + u32 num_rx_pkts = 0;
> > + unsigned int msg_obj;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + u32 val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> > +
> > + while (val & RECEIVE_OBJECT_BITS) {
> > + for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> > + msg_obj <= C_CAN_MSG_OBJ_RX_LAST; msg_obj++) {
> > + if (val & (1 << msg_obj)) {
> > + c_can_read_msg_object(dev, 0, msg_obj);
> > + num_rx_pkts++;
> > + quota--;
> > + }
> > + }
> > +
> > + val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> > + }
> > +
> > + return num_rx_pkts;
> > +}
> > +
> > +static int c_can_err(struct net_device *dev,
> > + enum c_can_bus_error_types error_type,
> > + int lec_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->reg_base-
> >error_counter);
> > + rx_err_passive = ((reg_err_counter & ERR_COUNTER_RP_MASK) >>
> > + ERR_COUNTER_RP_SHIFT);
> > +
> > + if (error_type & 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;
> > + if (bec.rxerr > 96)
> > + cf->data[1] = CAN_ERR_CRTL_RX_WARNING;
> > + if (bec.txerr > 96)
> > + cf->data[1] = CAN_ERR_CRTL_TX_WARNING;
> > + }
> > + if (error_type & 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;
> > + }
> > + if (error_type & 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_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > + can_bus_off(dev);
> > + }
> > +
> > + /* check for 'last error code' which tells us the
> > + * type of the last error to occur on the CAN bus
> > + */
> > + if (lec_type) {
> > + /* 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;
> > +
> > + if (lec_type & LEC_STUFF_ERROR) {
> > + dev_info(dev->dev.parent, "stuff error\n");
> > + cf->data[2] |= CAN_ERR_PROT_STUFF;
> > + }
> > + if (lec_type & LEC_FORM_ERROR) {
> > + dev_info(dev->dev.parent, "form error\n");
> > + cf->data[2] |= CAN_ERR_PROT_FORM;
> > + }
> > + if (lec_type & LEC_ACK_ERROR) {
> > + dev_info(dev->dev.parent, "ack error\n");
> > + cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> > + CAN_ERR_PROT_LOC_ACK_DEL);
> > + }
> > + if (lec_type & LEC_BIT1_ERROR) {
> > + dev_info(dev->dev.parent, "bit1 error\n");
> > + cf->data[2] |= CAN_ERR_PROT_BIT1;
> > + }
> > + if (lec_type & LEC_BIT0_ERROR) {
> > + dev_info(dev->dev.parent, "bit0 error\n");
> > + cf->data[2] |= CAN_ERR_PROT_BIT0;
> > + }
> > + if (lec_type & LEC_CRC_ERROR) {
> > + dev_info(dev->dev.parent, "CRC error\n");
> > + cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > + CAN_ERR_PROT_LOC_CRC_DEL);
> > + }
> > + }
> > +
> > + 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);
> > + enum c_can_bus_error_types error_type = C_CAN_NO_ERROR;
> > +
> > + irqstatus = priv->read_reg(priv, &priv->reg_base->ir);
> > +
> > + /* status events have the highest priority */
> > + if (irqstatus == STATUS_INTERRUPT) {
> > + priv->current_status = priv->read_reg(priv,
> > + &priv->reg_base->status);
> > +
> > + /* handle Tx/Rx events */
> > + if (priv->current_status & STATUS_TXOK)
> > + priv->write_reg(priv, &priv->reg_base->status,
> > + (priv->current_status & ~STATUS_TXOK));
> > +
> > + if (priv->current_status & STATUS_RXOK)
> > + priv->write_reg(priv, &priv->reg_base->status,
> > + (priv->current_status & ~STATUS_RXOK));
> > +
> > + /* handle bus error events */
> > + if (priv->current_status & STATUS_EWARN) {
> > + dev_info(dev->dev.parent,
> > + "entered error warning state\n");
> > + error_type = C_CAN_ERROR_WARNING;
> > + }
> > + if ((priv->current_status & STATUS_EPASS) &&
> > + (!(priv->last_status & STATUS_EPASS))) {
> > + dev_info(dev->dev.parent,
> > + "entered error passive state\n");
> > + error_type = C_CAN_ERROR_PASSIVE;
> > + }
> > + if ((priv->current_status & STATUS_BOFF) &&
> > + (!(priv->last_status & STATUS_BOFF))) {
> > + dev_info(dev->dev.parent,
> > + "entered bus off state\n");
> > + error_type = C_CAN_BUS_OFF;
> > + }
> > + if (priv->current_status & STATUS_LEC_MASK)
> > + lec_type = (priv->current_status & STATUS_LEC_MASK);
> > +
> > + /* handle bus recovery events */
> > + if ((!(priv->current_status & STATUS_EPASS)) &&
> > + (priv->last_status & STATUS_EPASS)) {
> > + dev_info(dev->dev.parent,
> > + "left error passive state\n");
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + }
> > + if ((!(priv->current_status & STATUS_BOFF)) &&
> > + (priv->last_status & STATUS_BOFF)) {
> > + dev_info(dev->dev.parent,
> > + "left bus off state\n");
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + }
> > +
> > + priv->last_status = priv->current_status;
> > +
> > + /* handle error on the bus */
> > + if (error_type != C_CAN_NO_ERROR)
> > + work_done += c_can_err(dev, error_type, 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));
> > + quota--;
> > + } 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);
> > + }
> > +
> > + if (work_done < quota) {
> > + napi_complete(napi);
> > + /* enable all IRQs */
> > + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > + }
> > +
> > + return work_done;
> > +}
> > +
> > +static irqreturn_t c_can_isr(int irq, void *dev_id) {
> > + struct net_device *dev = (struct net_device *)dev_id;
> > + struct c_can_priv *priv = netdev_priv(dev);
> > +
> > + /* disable all interrupts and schedule the NAPI */
> > + c_can_configure_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) {
> > + dev_err(dev->dev.parent, "failed to open can device\n");
> > + return err;
> > + }
> > +
> > + /* register interrupt handler */
> > + err = request_irq(dev->irq, &c_can_isr, priv->irq_flags, dev-
> >name,
> > + (void *)dev);
> > + if (err < 0) {
> > + dev_err(dev->dev.parent, "failed to attach interrupt\n");
> > + goto exit_irq_fail;
> > + }
> > +
> > + /* start the c_can controller */
> > + err = c_can_start(dev);
> > + if (err)
> > + goto exit_start_fail;
> > + napi_enable(&priv->napi);
> > +
> > + netif_start_queue(dev);
> > +
> > + return 0;
> > +
> > +exit_start_fail:
> > + free_irq(dev->irq, dev);
> > +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;
> > +}
> > +
> > +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,
> > +};
> > +
> > +static int c_can_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),
> DRV_NAME)) {
> > + 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_candev(sizeof(struct c_can_priv),
> C_CAN_MSG_OBJ_TX_NUM);
> > + if (!dev) {
> > + ret = -ENOMEM;
> > + goto exit_iounmap;
> > + }
> > +
> > + priv = netdev_priv(dev);
> > +
> > + priv->irq_flags = irq->flags;
> > + priv->reg_base = addr;
> > + priv->can.clock.freq = clk_get_rate(clk);
> > + priv->clk = clk;
> > +
> > + switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> > + case IORESOURCE_MEM_32BIT:
> > + priv->read_reg = c_can_read_reg_aligned_to_32bit;
> > + priv->write_reg = c_can_write_reg_aligned_to_32bit;
> > + break;
> > + case IORESOURCE_MEM_16BIT:
> > + default:
> > + priv->read_reg = c_can_read_reg_aligned_to_16bit;
> > + priv->write_reg = c_can_write_reg_aligned_to_16bit;
> > + break;
> > + }
> > +
> > + priv->dev = dev;
> > + priv->can.bittiming_const = &c_can_bittiming_const;
> > + priv->can.do_set_bittiming = c_can_set_bittiming;
> > + priv->can.do_get_state = c_can_get_state;
> > + 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;
> > +
> > + netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> > +
> > + dev->irq = irq->start;
> > + dev->flags |= IFF_ECHO; /* we support local echo */
> > + dev->netdev_ops = &c_can_netdev_ops;
> > + platform_set_drvdata(pdev, dev);
> > + SET_NETDEV_DEV(dev, &pdev->dev);
> > +
> > + ret = register_candev(dev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> > + DRV_NAME, ret);
> > + goto exit_free_device;
> > + }
> > +
> > + dev_info(&pdev->dev, "%s device registered (reg_base=%p,
> irq=%d)\n",
> > + DRV_NAME, priv->reg_base, dev->irq);
> > + return 0;
> > +
> > +exit_free_device:
> > + platform_set_drvdata(pdev, NULL);
> > + free_candev(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 c_can_remove(struct platform_device *pdev) {
> > + struct net_device *dev = platform_get_drvdata(pdev);
> > + struct c_can_priv *priv = netdev_priv(dev);
> > + struct resource *mem;
> > +
> > + /* disable all interrupts */
> > + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > + unregister_candev(dev);
> > + platform_set_drvdata(pdev, NULL);
> > +
> > + free_candev(dev);
> > + iounmap(priv->reg_base);
> > +
> > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + release_mem_region(mem->start, resource_size(mem));
> > +
> > + clk_put(priv->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver c_can_driver = {
> > + .driver = {
> > + .name = DRV_NAME,
> > + .owner = THIS_MODULE,
> > + },
> > + .probe = c_can_probe,
> > + .remove = c_can_remove,
> > +};
> > +
> > +static int __init c_can_init(void)
> > +{
> > + return platform_driver_register(&c_can_driver);
> > +}
> > +module_init(c_can_init);
> > +
> > +static void __exit c_can_exit(void)
> > +{
> > + platform_driver_unregister(&c_can_driver);
> > +}
> > +module_exit(c_can_exit);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("CAN bus driver for
> > +Bosch C_CAN controller");
>
>
> --
> 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 |
Regards,
Bhupesh
^ permalink raw reply
* Re: [PATCH] ipv4: Flush per-ns routing cache more sanely.
From: David Miller @ 2010-12-20 5:14 UTC (permalink / raw)
To: eric.dumazet; +Cc: ebiederm, netdev, daniel.lezcano
In-Reply-To: <1288121422.2652.14.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 26 Oct 2010 21:30:22 +0200
> Le mardi 26 octobre 2010 à 12:20 -0700, David Miller a écrit :
>> From: ebiederm@xmission.com (Eric W. Biederman)
>> Date: Tue, 26 Oct 2010 12:05:39 -0700
>>
>> >> @@ -999,7 +999,7 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
>> >> rt_cache_flush(dev_net(dev), 0);
>> >> break;
>> >> case NETDEV_UNREGISTER_BATCH:
>> >> - rt_cache_flush_batch();
>> >> + rt_cache_flush_batch(dev_net(dev));
>> >
>> > It still has this incorrect conversion in it.
>>
>> Sorry I missed that, what's the exact problem with it?
>
> Because the way _BATCH operation is performed, we call it once...
>
> rollback_registered_many() calls it for the first dev queued in the
> list.
>
> So it should be net independant.
Thanks Eric. I finally got back to fixing this issue and respinning
the patch.
Please review, in particular how I handled the RCU bits.
--------------------
ipv4: Flush per-ns routing cache more sanely.
Flush the routing cache only of entries that match the
network namespace in which the purge event occurred.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/route.h | 2 +-
net/ipv4/fib_frontend.c | 6 +++-
net/ipv4/route.c | 68 ++++++++++++++++++-----------------------------
3 files changed, 32 insertions(+), 44 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index 2700236..93e10c4 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -114,7 +114,7 @@ extern int ip_rt_init(void);
extern void ip_rt_redirect(__be32 old_gw, __be32 dst, __be32 new_gw,
__be32 src, struct net_device *dev);
extern void rt_cache_flush(struct net *net, int how);
-extern void rt_cache_flush_batch(void);
+extern void rt_cache_flush_batch(struct net *net);
extern int __ip_route_output_key(struct net *, struct rtable **, const struct flowi *flp);
extern int ip_route_output_key(struct net *, struct rtable **, struct flowi *flp);
extern int ip_route_output_flow(struct net *, struct rtable **rp, struct flowi *flp, struct sock *sk, int flags);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index d3a1112..9f8bb68 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -987,7 +987,11 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
rt_cache_flush(dev_net(dev), 0);
break;
case NETDEV_UNREGISTER_BATCH:
- rt_cache_flush_batch();
+ /* The batch unregister is only called on the first
+ * device in the list of devices being unregistered.
+ * Therefore we should not pass dev_net(dev) in here.
+ */
+ rt_cache_flush_batch(NULL);
break;
}
return NOTIFY_DONE;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ae52096..7c87d8e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -717,13 +717,15 @@ static inline int rt_is_expired(struct rtable *rth)
* Can be called by a softirq or a process.
* In the later case, we want to be reschedule if necessary
*/
-static void rt_do_flush(int process_context)
+static void rt_do_flush(struct net *net, int process_context)
{
unsigned int i;
struct rtable *rth, *next;
- struct rtable * tail;
for (i = 0; i <= rt_hash_mask; i++) {
+ struct rtable __rcu **pprev;
+ struct rtable *list;
+
if (process_context && need_resched())
cond_resched();
rth = rcu_dereference_raw(rt_hash_table[i].chain);
@@ -731,52 +733,34 @@ static void rt_do_flush(int process_context)
continue;
spin_lock_bh(rt_hash_lock_addr(i));
-#ifdef CONFIG_NET_NS
- {
- struct rtable __rcu **prev;
- struct rtable *p;
- rth = rcu_dereference_protected(rt_hash_table[i].chain,
+ list = NULL;
+ pprev = &rt_hash_table[i].chain;
+ rth = rcu_dereference_protected(*pprev,
lockdep_is_held(rt_hash_lock_addr(i)));
- /* defer releasing the head of the list after spin_unlock */
- for (tail = rth; tail;
- tail = rcu_dereference_protected(tail->dst.rt_next,
- lockdep_is_held(rt_hash_lock_addr(i))))
- if (!rt_is_expired(tail))
- break;
- if (rth != tail)
- rt_hash_table[i].chain = tail;
-
- /* call rt_free on entries after the tail requiring flush */
- prev = &rt_hash_table[i].chain;
- for (p = rcu_dereference_protected(*prev,
- lockdep_is_held(rt_hash_lock_addr(i)));
- p != NULL;
- p = next) {
- next = rcu_dereference_protected(p->dst.rt_next,
+ while (rth) {
+ next = rcu_dereference_protected(rth->dst.rt_next,
lockdep_is_held(rt_hash_lock_addr(i)));
- if (!rt_is_expired(p)) {
- prev = &p->dst.rt_next;
+
+ if (!net ||
+ net_eq(dev_net(rth->dst.dev), net)) {
+ rcu_assign_pointer(*pprev, next);
+ rcu_assign_pointer(rth->dst.rt_next, list);
+ list = rth;
} else {
- *prev = next;
- rt_free(p);
+ pprev = &rth->dst.rt_next;
}
+ rth = next;
}
- }
-#else
- rth = rcu_dereference_protected(rt_hash_table[i].chain,
- lockdep_is_held(rt_hash_lock_addr(i)));
- rcu_assign_pointer(rt_hash_table[i].chain, NULL);
- tail = NULL;
-#endif
+
spin_unlock_bh(rt_hash_lock_addr(i));
- for (; rth != tail; rth = next) {
- next = rcu_dereference_protected(rth->dst.rt_next, 1);
- rt_free(rth);
- }
- }
+ for (; list; list = next) {
+ next = rcu_dereference_protected(list->dst.rt_next, 1);
+ rt_free(list);
+ }
+ }
}
/*
@@ -922,13 +906,13 @@ void rt_cache_flush(struct net *net, int delay)
{
rt_cache_invalidate(net);
if (delay >= 0)
- rt_do_flush(!in_softirq());
+ rt_do_flush(net, !in_softirq());
}
/* Flush previous cache invalidated entries from the cache */
-void rt_cache_flush_batch(void)
+void rt_cache_flush_batch(struct net *net)
{
- rt_do_flush(!in_softirq());
+ rt_do_flush(net, !in_softirq());
}
static void rt_emergency_hash_rebuild(struct net *net)
--
1.7.3.4
^ permalink raw reply related
* Re: ip rule and/or route problem in 2.6.37-rc5+
From: David Miller @ 2010-12-20 5:42 UTC (permalink / raw)
To: greearb; +Cc: netdev, therbert
In-Reply-To: <4D07E18F.30703@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Thu, 09 Dec 2010 22:19:06 -0800
> On 12/09/2010 05:06 PM, Ben Greear wrote:
>>
>> This problem appears to have happened between 2.6.36.1 and 2.6.37-rc2.
>> We haven't fully bisected the problem yet.
>>
>>
>> The basic test:
>>
>> * one normal interface using DHCP
>> * A second interface specified to use it's own routing table.
>> * 'ip rules' to determine behaviour.
>>
>> After running these commands abelow, the system can no longer
>> route out it's normal interface. It appears that the final line
>> is the one that messes things up. If you flush table 10001 after
>> that, things start working again.
>>
>> The 'pref 20' rule is also important. It should not have
>> any affect on this ping, but it appears that it does, somehow.
>> If you remove it, the problem also goes away, regardless of
>> the routes in table 10001.
>>
>>
>> ip rule add pref 512 lookup local
>> ip rule del pref 0 lookup local
>> ip link set eth2 up
>> ip -4 addr add 172.16.0.102/24 broadcast 172.16.0.255 dev eth2
>> ip rule add to 172.16.0.102 iif eth2 lookup local pref 10
>> ip rule add iif eth2 lookup 10001 pref 20
>> ip route add 172.16.0.0/24 dev eth2 table 10001
>> ip route add unreachable 0/0 table 10001
>
> Seems this is the commit that broke this behaviour:
>
> 4465b469008bc03b98a1b8df4e9ae501b6c69d4b is first bad commit
> commit 4465b469008bc03b98a1b8df4e9ae501b6c69d4b
> Author: Tom Herbert <therbert@google.com>
> Date: Sun May 23 19:54:12 2010 +0000
>
> ipv4: Allow configuring subnets as local addresses
Tom, please acknowledge this regression you've added to the tree.
^ permalink raw reply
* Re: [PATCH] net: increase skb->users instead of skb_clone()
From: David Miller @ 2010-12-20 5:50 UTC (permalink / raw)
To: xiaosuo
Cc: eric.dumazet, therbert, jpirko, fenghua.yu, junchangwang,
xinan.tang, netdev
In-Reply-To: <1292479045-3136-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 16 Dec 2010 13:57:25 +0800
> In dev_queue_xmit_nit(), we have to clone skbs as we need to mangle skbs,
> however, we don't need to clone skbs for all the packet_types.
>
> Except for the first packet_type, we increase skb->users instead of
> skb_clone().
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] vxge: add missing flush of reset_task
From: David Miller @ 2010-12-20 5:54 UTC (permalink / raw)
To: jon.mason; +Cc: tj, netdev, linux-kernel
In-Reply-To: <20101215142859.GA13516@exar.com>
From: Jon Mason <jon.mason@exar.com>
Date: Wed, 15 Dec 2010 08:29:00 -0600
> On Wed, Dec 15, 2010 at 06:03:29AM -0800, Tejun Heo wrote:
>> Commit 6e07ebd84 (drivers/net: remove unnecessary
>> flush_scheduled_work() calls) incorrectly removed the flush call
>> without replacing it with the appropriate work specific operation.
>> Fix it by flushing vdev->reset_task explicitly.
>>
>> Pointed out by Jon Mason.
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
>> Cc: Jon Mason <jon.mason@exar.com>
> Acked-by: Jon Mason <jon.mason@exar.com>
I'll apply this, thanks.
^ permalink raw reply
* Re: [PATCH 1/3] net: kill unused macros
From: David Miller @ 2010-12-20 5:59 UTC (permalink / raw)
To: shanwei
Cc: paul.moore, joe, jslaby, eric.dumazet, tj, ebiederm, adobriyan,
herbert, nhorman, amwang, netdev
In-Reply-To: <4D085FB2.5050502@cn.fujitsu.com>
From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Wed, 15 Dec 2010 14:26:58 +0800
> These macros never be used, so remove them.
>
> Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
Applied, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox