Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-26] cxgb4vf: recover from failure in cxgb4vf_open()
From: David Miller @ 2011-01-11 23:44 UTC (permalink / raw)
  To: leedom; +Cc: netdev
In-Reply-To: <1294783079-7481-1-git-send-email-leedom@chelsio.com>

From: Casey Leedom <leedom@chelsio.com>
Date: Tue, 11 Jan 2011 13:57:59 -0800

> If the Link Start fails in cxgb4vf_open(), we need to back out any state
> that we've built up ...
> 
> Signed-off-by: Casey Leedom <leedom@chelsio.com>

Applied, thank you.

^ permalink raw reply

* Re: [RFC] sched: CHOKe packet scheduler (v0.4)
From: Stephen Hemminger @ 2011-01-11 23:48 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1294727650.4148.20.camel@edumazet-laptop>

On Tue, 11 Jan 2011 07:34:10 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le mardi 11 janvier 2011 à 07:18 +0100, Eric Dumazet a écrit :
> > Le lundi 10 janvier 2011 à 17:10 -0800, Stephen Hemminger a écrit :
> > > OK, put that in.
> > 
> > Thanks !
> > 
> > > > 
> > > > Here we missed :
> > > > 
> > > > 		q->tab = ntab;
> > > > 
> > 
> > We also need to change 
> > 
> > .peek       =   qdisc_peek_head,
> > 
> > to
> > 
> > .peek = choke_peek_head,
> > 
> > static struct sk_buff *choke_peek_head(struct Qdisc *sch)
> > {
> > 	struct choke_sched_data *q = qdisc_priv(sch);
> > 
> > 	return (q->head != q->tail) ? q->tab[q->head] : NULL;
> > }
> > 
> > 
> 
> 
> And to correctly work with CBQ (at least...), we need to update 
> sch->q.qlen = choke_len(q) - q->holes;
> in dequeue() and enqueue()
> 
> Here is the version I successfully tested, with 30000 packets in
> queue :)
> 
> qdisc choke 11: parent 1:11 limit 70000b min 10000b max 30000b ewma 1 Plog 16 Scell_log 11
>  Sent 62099201 bytes 112920 pkt (dropped 367712, overlimits 282668 requeues 0) 
>  rate 21344Kbit 4851pps backlog 39877589b 30001p requeues 0 
>   marked 0 early 282668 pdrop 0 other 0

Maybe we should take over one of the red counters for the probabilistic match drop.


^ permalink raw reply

* Re: [RFC] sched: CHOKe packet scheduler (v0.4)
From: Eric Dumazet @ 2011-01-12  0:04 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20110111154806.6525e210@s6510>

Le mardi 11 janvier 2011 à 15:48 -0800, Stephen Hemminger a écrit :
> On Tue, 11 Jan 2011 07:34:10 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > qdisc choke 11: parent 1:11 limit 70000b min 10000b max 30000b ewma 1 Plog 16 Scell_log 11
> >  Sent 62099201 bytes 112920 pkt (dropped 367712, overlimits 282668 requeues 0) 
> >  rate 21344Kbit 4851pps backlog 39877589b 30001p requeues 0 
> >   marked 0 early 282668 pdrop 0 other 0
> 
> Maybe we should take over one of the red counters for the probabilistic match drop.
> 

aka chokedrop ;)

Anyway, iproute2 should be tweaked a bit, since limit/min/max are
packets, not bytes like RED. It would be nice to print probability
too ;)

AFAIK, requeue counter is not anymore used (On leaves at least), we
could re-use it for chokedrop

----
qdisc choke 11: parent 1:11 limit 70000p min 10000p max 30000p proba 0.08
  Sent 62099201 bytes 112920 pkt (dropped 367712, overlimits 282668 chokedrop 999) 
  rate 21344Kbit 4851pps backlog 39877589b 30001p chokedrop 999
  marked 0 early 282668 pdrop 0 other 0
 



^ permalink raw reply

* Re: [GIT PULL nf-next-2.6] ipvs namespaces
From: Pablo Neira Ayuso @ 2011-01-12  0:15 UTC (permalink / raw)
  To: Simon Horman
  Cc: David Miller, netfilter-devel, lvs-devel, netdev, kaber, ja,
	hans.schillstrom, davem
In-Reply-To: <20110111230118.GK3848@verge.net.au>

On 12/01/11 00:01, Simon Horman wrote:
> On Tue, Jan 11, 2011 at 02:34:45PM -0800, David Miller wrote:
>> From: Simon Horman <horms@verge.net.au>
>> Date: Tue, 11 Jan 2011 15:21:14 +0900
>>
>>> Patrick seems to be rather quiet (perhaps he is on holidays?).
>>> Could you consider taking these changes through your tree as
>>> it would be nice to get this feature in 2.6.38?
>>
>> Patrick seems to be coming back up to speed.  And also we're trying
>> to setup a situation where Pablo is able to pick up the slack when
>> Patrick is away or busy.
>>
>> So it's let's try to get your changes merged that way. :-)
> 
> No problem, lets do that :-)

JFYI: I'll start iterating through the pending patches (since Dec 16th)
to apply them to 2.6.38-rc tomorrow.

^ permalink raw reply

* Re: [GIT PULL nf-next-2.6] ipvs namespaces
From: Simon Horman @ 2011-01-12  0:20 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: David Miller, netfilter-devel, lvs-devel, netdev, kaber, ja,
	hans.schillstrom, davem
In-Reply-To: <4D2CF289.1@netfilter.org>

On Wed, Jan 12, 2011 at 01:15:05AM +0100, Pablo Neira Ayuso wrote:
> On 12/01/11 00:01, Simon Horman wrote:
> > On Tue, Jan 11, 2011 at 02:34:45PM -0800, David Miller wrote:
> >> From: Simon Horman <horms@verge.net.au>
> >> Date: Tue, 11 Jan 2011 15:21:14 +0900
> >>
> >>> Patrick seems to be rather quiet (perhaps he is on holidays?).
> >>> Could you consider taking these changes through your tree as
> >>> it would be nice to get this feature in 2.6.38?
> >>
> >> Patrick seems to be coming back up to speed.  And also we're trying
> >> to setup a situation where Pablo is able to pick up the slack when
> >> Patrick is away or busy.
> >>
> >> So it's let's try to get your changes merged that way. :-)
> > 
> > No problem, lets do that :-)
> 
> JFYI: I'll start iterating through the pending patches (since Dec 16th)
> to apply them to 2.6.38-rc tomorrow.

Thanks Pablo.

^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2011-01-12  0:24 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, netfilter-devel, linux-wireless, linux-kernel


Mostly driver stuff (as usual), a TCP bind fix, some checksum
offloading cures, and random other small bits all over.

1) Fix signedness bugs in phonet, from Dan Carpenter.

2) Sequence number checking fixes in DCCP from Samuel Jero and
   Gerrit Renker.

3) Support both ports of FEC ethernet device properly, from
   Shawn Guo.

4) Memory leak fix in hamradio and ATM ambassador driver from Jesper
   Juhl.

5) MSI interrupt and statistic handling fixes in bnx2x from Vladislav
   Zolotarov and Eilon Greenstein.

6) Autonegotiation and VLAN fixes in sky2 from Stephen Hemminger.

7) Power management fixes in forcedeth from Rafael J. Wysocki.

8) Fix handling of NLM_F_ROOT | NLM_F_MATCH (can be mistaken as
   a NLM_F_DUMP request) in genetlink.  From Jan Engelhardt.

9) Kernel doc fixes in net/sock.h and net/core/filter.c from Randy
   Dunlap

10) Do PHY init, and thus firmware request, at ->open() time to
    workaround bootup 60 second delay when r8169 is built statically
    into the kernel.  From Francois Romieu.

11) Checksumming offload flags are handled improperly, in particular
    when VLAN's are nested.  From Jesse Gross.

12) Various Intel wired driver fixes from Bruce Allan, Jeff Kirsher,
    Dirk Brandewie, Yi Zou, and Alexander Duyck.

13) Software interrupts are disabled way too long when reading counters
    (for "iptables -L" output, for example).  Fix from Eric Dumazet.

14) bfin_mac driver has to disable checksum offloading when a writeback
    cache is in use, since corrupt packets can result, fix from
    Sonic Zhang.

15) Firmware version detection and ethtool diag dixes in qlcnic from
    Amit Kumar Salecha and Sony Chacko.

16) Mailbox register coherency, and ->open() failure unwinding fixes
    in cxgb4vf from Casey Leedom.

17) On user copy failure, we erroneously leave the connection request
    parameter size set, in CAIF protocol.  Fix from Dan Rosenberg.

18) MLX4 driver needs to be able to allocate different numbers of
    TX vs. RX queues, add a helper for that and use it.  From Tom
    Herbert.

19) Only HTB scheduler handles packet counting in statistics properly
    wrt. segmented SKBs.  Fix this by creating a helper function and
    using it everywhere.  From Eric Dumazet.

20) Firewire needs to be able to invalidate specific ARP entries since
    it uses ARP to discover private info about firewiare network nodes.
    From Maxim Levitsky.

21) IPV6 not handled properly in CAIF, fix from Sjur Braendeland and
    Kumar Sanghvi.

22) Firmware parsing function in r8169 needs some minor tweaks, from
    Hayes Wang.

23) TCP binding bug fix from Eric Dumazet.

24) ehea RX ring initialization fix on device up from Breno Leitao.

25) AUTH truncation bug fixes in IPSEC from Nicolas Dichtel.

26) AH protocol header parsing needs to reload pointers after
    skb COW, also from Nicolas Dichtel.

27) Fix netfilter conntrack race between table dumping and destroy of
    entries, from Stephen Hemminger.

28) RCU annotation addition to bridge netfilter broke broute table,
    fix from Florian Westphal.

Please pull, thanks a lot!

The following changes since commit 0c21e3aaf6ae85bee804a325aa29c325209180fd:

  Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/hch/hfsplus (2011-01-07 17:16:27 -0800)

are available in the git repository at:

  master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.git master

Alexander Duyck (3):
      ixgbe: cleanup flow director hash computation to improve performance
      ixgbe: further flow director performance optimizations
      ixgbe: update ntuple filter configuration

Breno Leitao (1):
      ehea: Increase the skb array usage

Bruce Allan (6):
      e1000e: cleanup variables set but not used
      e1000e: convert calls of ops.[read|write]_reg to e1e_[r|w]phy
      e1000e: properly bounds-check string functions
      e1000e: use either_crc_le() rather than re-write it
      e1000e: power off PHY after reset when interface is down
      e1000e: add custom set_d[0|3]_lplu_state function pointer for 82574

Casey Leedom (2):
      cxgb4vf: fix mailbox data/control coherency domain race
      cxgb4vf: recover from failure in cxgb4vf_open()

Changli Gao (1):
      net: ppp: use {get,put}_unaligned_be{16,32}

Dan Carpenter (1):
      phonet: some signedness bugs

Dan Rosenberg (1):
      caif: don't set connection request param size before copying data

Dang Hongwu (1):
      ah: reload pointers to skb data after calling skb_cow_data()

David S. Miller (2):
      Merge branch 'dccp' of git://eden-feed.erg.abdn.ac.uk/net-next-2.6
      Merge branch 'master' of git://1984.lsi.us.es/net-2.6

Dirk Brandewie (1):
      e1000: Add support for the CE4100 reference platform

Eric Dumazet (3):
      netfilter: x_tables: dont block BH while reading counters
      net_sched: factorize qdisc stats handling
      tcp: disallow bind() to reuse addr/port

Florian Westphal (1):
      netfilter: ebtables: make broute table work again

Gerrit Renker (1):
      dccp: make upper bound for seq_window consistent on 32/64 bit

Jan Engelhardt (1):
      netlink: test for all flags of the NLM_F_DUMP composite

Jesper Juhl (2):
      hamradio: Resolve memory leak due to missing firmware release in add_mcs()
      Madge Ambassador ATM Adapter driver: Always release_firmware() in ucode_init() and don't leak memory.

Jesse Gross (6):
      net offloading: Accept NETIF_F_HW_CSUM for all protocols.
      net offloading: Generalize netif_get_vlan_features().
      net offloading: Pass features into netif_needs_gso().
      net offloading: Convert dev_gso_segment() to use precomputed features.
      net offloading: Convert skb_need_linearize() to use precomputed features.
      net offloading: Convert checksums to use centrally computed features.

Ken Kawasaki (1):
      pcnet_cs: add new_id

Kumar Sanghvi (1):
      CAIF: Fix IPv6 support in receive path for GPRS/3G

Maxim Levitsky (1):
      arp: allow to invalidate specific ARP entries

Mike Frysinger (4):
      netdev: bfin_mac: clean up printk messages
      netdev: bfin_mac: mark setup_system_regs as static
      netdev: bfin_mac: drop unused Mac data
      netdev: bfin_mac: let boards set vlan masks

Nicolas Dichtel (2):
      xfrm: check trunc_len in XFRMA_ALG_AUTH_TRUNC
      ah: update maximum truncated ICV length

Rafael J. Wysocki (1):
      forcedeth: Do not use legacy PCI power management

Randy Dunlap (2):
      net/sock.h: make some fields private to fix kernel-doc warning(s)
      net: fix kernel-doc warning in core/filter.c

Samuel Jero (2):
      dccp: fix return value for sequence-invalid packets
      dccp: fix bug in updating the GSR

Shawn Guo (6):
      net/fec: fix MMFR_OP type in fec_enet_mdio_write
      net/fec: remove the use of "index" which is legacy
      net/fec: add mac field into platform data and consolidate fec_get_mac
      net/fec: improve pm for better suspend/resume
      net/fec: add dual fec support for mx28
      net/fec: remove config FEC2 as it's used nowhere

Sonic Zhang (1):
      netdev: bfin_mac: disable hardware checksum if writeback cache is enabled

Sony Chacko (1):
      qlcnic: fix ethtool diagnostics test

Stephen Hemminger (3):
      sky2: fix limited auto negotiation
      sky2: convert to new VLAN model (v0.2)
      netfilter: fix race in conntrack between dump_table and destroy

Tom Herbert (2):
      net: Add alloc_netdev_mqs function
      mlx4: Call alloc_etherdev to allocate RX and TX queues

Vladislav Zolotarov (4):
      bnx2x: Don't prevent RSS configuration in INT#x and MSI interrupt modes.
      bnx2x: registers dump fixes
      bnx2x: Move to D0 before clearing MSI/MSI-X configuration.
      bnx2x: Fix the race on bp->stats_pending.

Yi Zou (1):
      ixgbe: make sure per Rx queue is disabled before unmapping the receive buffer

amit salecha (2):
      qlcnic: fix flash fw version read
      qlcnic: change module parameter permissions

françois romieu (1):
      r8169: delay phy init until device opens.

hayeswang (1):
      net/r8169: Update the function of parsing firmware

 Documentation/networking/dccp.txt    |    1 +
 drivers/atm/ambassador.c             |   19 +-
 drivers/net/Kconfig                  |    9 +-
 drivers/net/bfin_mac.c               |   74 ++--
 drivers/net/bfin_mac.h               |   11 +-
 drivers/net/bnx2x/bnx2x.h            |    1 +
 drivers/net/bnx2x/bnx2x_dump.h       |  988 +++++++++++++++++++--------------
 drivers/net/bnx2x/bnx2x_ethtool.c    |   22 +-
 drivers/net/bnx2x/bnx2x_init.h       |  220 ++++++++
 drivers/net/bnx2x/bnx2x_main.c       |   70 +--
 drivers/net/bnx2x/bnx2x_reg.h        |   74 +++
 drivers/net/bnx2x/bnx2x_stats.c      |    5 +
 drivers/net/cxgb4vf/cxgb4vf_main.c   |   15 +-
 drivers/net/cxgb4vf/t4vf_hw.c        |   11 +
 drivers/net/e1000/e1000_hw.c         |  328 +++++++++---
 drivers/net/e1000/e1000_hw.h         |   59 ++-
 drivers/net/e1000/e1000_main.c       |   35 ++
 drivers/net/e1000/e1000_osdep.h      |   19 +-
 drivers/net/e1000e/82571.c           |   77 +++-
 drivers/net/e1000e/e1000.h           |    3 +
 drivers/net/e1000e/es2lan.c          |    4 +-
 drivers/net/e1000e/ethtool.c         |   54 ++-
 drivers/net/e1000e/hw.h              |    1 +
 drivers/net/e1000e/ich8lan.c         |   77 +--
 drivers/net/e1000e/lib.c             |    3 +-
 drivers/net/e1000e/netdev.c          |   53 ++-
 drivers/net/e1000e/phy.c             |   40 +-
 drivers/net/ehea/ehea.h              |    2 +-
 drivers/net/ehea/ehea_main.c         |    6 +-
 drivers/net/fec.c                    |  248 ++++++---
 drivers/net/fec.h                    |    5 +-
 drivers/net/forcedeth.c              |   34 +-
 drivers/net/hamradio/yam.c           |    4 +-
 drivers/net/ixgbe/ixgbe.h            |   21 +-
 drivers/net/ixgbe/ixgbe_82599.c      |  749 ++++++++++----------------
 drivers/net/ixgbe/ixgbe_ethtool.c    |  142 ++++--
 drivers/net/ixgbe/ixgbe_main.c       |  169 +++++--
 drivers/net/ixgbe/ixgbe_type.h       |   91 ++--
 drivers/net/mlx4/en_netdev.c         |    3 +-
 drivers/net/pcmcia/pcnet_cs.c        |    1 +
 drivers/net/ppp_async.c              |   10 +-
 drivers/net/ppp_deflate.c            |    9 +-
 drivers/net/ppp_generic.c            |    9 +-
 drivers/net/ppp_mppe.c               |    7 +-
 drivers/net/ppp_synctty.c            |    3 +-
 drivers/net/qlcnic/qlcnic.h          |   24 +-
 drivers/net/qlcnic/qlcnic_ethtool.c  |    2 +-
 drivers/net/qlcnic/qlcnic_init.c     |   63 +++-
 drivers/net/qlcnic/qlcnic_main.c     |   10 +-
 drivers/net/r8169.c                  |  143 +++++-
 drivers/net/sky2.c                   |  143 +++---
 drivers/net/sky2.h                   |    6 +-
 drivers/net/xen-netfront.c           |    2 +-
 include/linux/bfin_mac.h             |    1 +
 include/linux/etherdevice.h          |    4 +-
 include/linux/fec.h                  |    3 +
 include/linux/if_bridge.h            |    2 +-
 include/linux/netdevice.h            |   24 +-
 include/linux/netfilter/x_tables.h   |   10 +-
 include/net/ah.h                     |    2 +-
 include/net/arp.h                    |    1 +
 include/net/phonet/phonet.h          |    4 +-
 include/net/sch_generic.h            |   20 +-
 include/net/sock.h                   |    4 +
 net/caif/caif_socket.c               |    2 +-
 net/caif/chnl_net.c                  |   18 +-
 net/core/dev.c                       |  149 +++---
 net/core/filter.c                    |    2 +-
 net/core/rtnetlink.c                 |    2 +-
 net/dccp/dccp.h                      |    3 +-
 net/dccp/input.c                     |    2 +-
 net/dccp/sysctl.c                    |    4 +-
 net/ethernet/eth.c                   |   12 +-
 net/ipv4/ah4.c                       |    7 +-
 net/ipv4/arp.c                       |   29 +-
 net/ipv4/inet_connection_sock.c      |    5 +-
 net/ipv4/inet_diag.c                 |    2 +-
 net/ipv4/netfilter/arp_tables.c      |   45 +--
 net/ipv4/netfilter/ip_tables.c       |   45 +--
 net/ipv6/ah6.c                       |    8 +-
 net/ipv6/inet6_connection_sock.c     |    2 +-
 net/ipv6/netfilter/ip6_tables.c      |   45 +--
 net/netfilter/nf_conntrack_netlink.c |   18 +-
 net/netfilter/x_tables.c             |    3 +-
 net/netlink/genetlink.c              |    2 +-
 net/phonet/af_phonet.c               |    6 +-
 net/sched/act_csum.c                 |    3 +-
 net/sched/act_ipt.c                  |    3 +-
 net/sched/act_mirred.c               |    3 +-
 net/sched/act_nat.c                  |    3 +-
 net/sched/act_pedit.c                |    3 +-
 net/sched/act_police.c               |    3 +-
 net/sched/act_simple.c               |    3 +-
 net/sched/act_skbedit.c              |    3 +-
 net/sched/sch_atm.c                  |    6 +-
 net/sched/sch_cbq.c                  |    6 +-
 net/sched/sch_drr.c                  |    8 +-
 net/sched/sch_dsmark.c               |    3 +-
 net/sched/sch_hfsc.c                 |    6 +-
 net/sched/sch_htb.c                  |   17 +-
 net/sched/sch_ingress.c              |    3 +-
 net/sched/sch_multiq.c               |    3 +-
 net/sched/sch_netem.c                |    6 +-
 net/sched/sch_prio.c                 |    3 +-
 net/sched/sch_red.c                  |    3 +-
 net/sched/sch_sfq.c                  |    3 +-
 net/sched/sch_tbf.c                  |    3 +-
 net/sched/sch_teql.c                 |    3 +-
 net/xfrm/xfrm_user.c                 |    6 +-
 109 files changed, 2901 insertions(+), 1867 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* TCP connections stall when station re-associates.
From: Ben Greear @ 2011-01-12  1:02 UTC (permalink / raw)
  To: NetDev, linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org


We're seeing something funny while testing ath9k and ath5k with
multiple VIFs.  We are using send-to-self routing rules
and have a TCP connection going between two station interfaces
connected to the same AP.  Kernel is ~2.6.37 (wireless-testing from
a few days ago, plus some local patches).  We tried commercial
APs and ath9k/ath5k APs running 2.6.37 kernel and hostap with
similar results.

The problem is that when the stations loose association and
re-associate (and maybe other times as well), the TCP connections
often hang.  I see buffers in the sendq in netstat, and retransmits
on the wire, but it seems to make no progress.

Here's a pair of sockets with one side in the state:

tcp        0   7312 7.7.1.54:33513              7.7.1.56:33514              ESTABLISHED
tcp        0      0 7.7.1.56:33514              7.7.1.54:33513              ESTABLISHED

Here's a brief snippet from the interface with 7.7.1.56 on it:

  51.517954     7.7.1.56 -> 7.7.1.54     TCP 33514 > 33513 [PSH, ACK] Seq=64229 Ack=1 Win=63 Len=12 TSV=8209159 TSER=7621148
  51.521719     7.7.1.54 -> 7.7.1.56     TCP 33513 > 33514 [ACK] Seq=13 Ack=64241 Win=377 Len=0 TSV=8209162 TSER=8209159
  52.279788     7.7.1.54 -> 7.7.1.56     TCP [TCP Retransmission] 33513 > 33514 [PSH, ACK] Seq=1 Ack=64241 Win=377 Len=12 TSV=8209920 TSER=8209159

The TCP connection is trying to run at 9.6kbps, sending 1460 bytes
per 'send' call.

If I start a UDP connection on the same pair of ports it
runs fine, even while the TCP connection is hung.

I am curious if anyone has any suggestions for debugging
this further, for instance, some way to see why the
TCP connection is not making forward progress.

Thanks,
Ben

-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] [RFC] ath9k: Fix reporting of RX STBC streams to userspace
From: Felix Fietkau @ 2011-01-12  1:19 UTC (permalink / raw)
  To: Bernhard Walle
  Cc: netdev, linux-wireless, linville, jmalinen, senthilkumar,
	ath9k-devel, vasanth, linux-kernel
In-Reply-To: <1294765194-30542-1-git-send-email-walle@corscience.de>

On 2011-01-11 9:59 AM, Bernhard Walle wrote:
> While the driver reports
>
>          ath: TX streams 2, RX streams: 2
>
> in the kernel log (with ATH_DBG_CONFIG set in the debug module
> parameter), "iw list" only reported
>
> [...]
>          Capabilities: 0x12ce
>                  HT20/HT40
>                  SM Power Save disabled
>                  RX HT40 SGI
>                  TX STBC
>                  RX STBC 1-streams
> [...]
>
> The driver seems to set the value as flag while the iw tool interprets
> it as number. This patch fixes that.
>
> Signed-off-by: Bernhard Walle<walle@corscience.de>
> ---
>   drivers/net/wireless/ath/ath9k/init.c |   12 ++++++------
>   1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index 639dc93..935b6c3 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -215,17 +215,17 @@ static void setup_ht_cap(struct ath_softc *sc,
>   	else
>   		max_streams = 2;
>
> -	if (AR_SREV_9280_20_OR_LATER(ah)) {
> -		if (max_streams>= 2)
> -			ht_info->cap |= IEEE80211_HT_CAP_TX_STBC;
> -		ht_info->cap |= (1<<  IEEE80211_HT_CAP_RX_STBC_SHIFT);
> -	}
> -
>   	/* set up supported mcs set */
>   	memset(&ht_info->mcs, 0, sizeof(ht_info->mcs));
>   	tx_streams = ath9k_cmn_count_streams(common->tx_chainmask, max_streams);
>   	rx_streams = ath9k_cmn_count_streams(common->rx_chainmask, max_streams);
>
> +	if (AR_SREV_9280_20_OR_LATER(ah)) {
> +		if (max_streams>= 2)
> +			ht_info->cap |= IEEE80211_HT_CAP_TX_STBC;
> +		ht_info->cap |= (rx_streams<<  IEEE80211_HT_CAP_RX_STBC_SHIFT);
> +	}
> +
Are you sure the card can actually receive multiple streams via STBC? As 
far as I know, only one stream can be received properly via STBC, more 
streams have to be sent without it.

- Felix

^ permalink raw reply

* Re: panic in tg3 driver
From: Matt Carlson @ 2011-01-12  3:06 UTC (permalink / raw)
  To: Stephen Clark
  Cc: Matthew Carlson, Linux Kernel Network Developers, Michael Chan
In-Reply-To: <4D2C64EF.1080905@earthlink.net>

On Tue, Jan 11, 2011 at 06:10:55AM -0800, Stephen Clark wrote:
> On 01/10/2011 09:00 PM, Matt Carlson wrote:
> > On Mon, Jan 10, 2011 at 12:04:34PM -0800, Stephen Clark wrote:
> >    
> >> On 01/10/2011 02:22 PM, Matt Carlson wrote:
> >>      
> >>> On Sun, Jan 09, 2011 at 02:30:50PM -0800, Stephen Clark wrote:
> >>>
> >>>        
> >>>> On 01/04/2011 09:54 AM, Stephen Clark wrote:
> >>>>
> >>>>          
> >>>>> Hello,
> >>>>>
> >>>>>
> >>>>> The hardware is an Acrosser AR-M0898B micro box.
> >>>>>    lspci
> >>>>> 00:00.0 Host bridge: VIA Technologies, Inc. CN700/VN800/P4M800CE/Pro
> >>>>> Host Bridge
> >>>>> 00:00.1 Host bridge: VIA Technologies, Inc. CN700/VN800/P4M800CE/Pro
> >>>>> Host Bridge
> >>>>> 00:00.2 Host bridge: VIA Technologies, Inc. CN700/VN800/P4M800CE/Pro
> >>>>> Host Bridge
> >>>>> 00:00.3 Host bridge: VIA Technologies, Inc. PT890 Host Bridge
> >>>>> 00:00.4 Host bridge: VIA Technologies, Inc. CN700/VN800/P4M800CE/Pro
> >>>>> Host Bridge
> >>>>> 00:00.7 Host bridge: VIA Technologies, Inc. CN700/VN800/P4M800CE/Pro
> >>>>> Host Bridge
> >>>>> 00:01.0 PCI bridge: VIA Technologies, Inc. VT8237/VX700 PCI Bridge
> >>>>> 00:0f.0 IDE interface: VIA Technologies, Inc. VT8251 Serial ATA
> >>>>> Controller (rev
> >>>>> 20)
> >>>>> 00:0f.1 IDE interface: VIA Technologies, Inc.
> >>>>> VT82C586A/B/VT82C686/A/B/VT823x/A/
> >>>>> C PIPC Bus Master IDE (rev 07)
> >>>>> 00:10.0 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1
> >>>>> Controller
> >>>>>    (rev 91)
> >>>>> 00:10.1 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1
> >>>>> Controller
> >>>>>    (rev 91)
> >>>>> 00:10.2 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1
> >>>>> Controller
> >>>>>    (rev 91)
> >>>>> 00:10.3 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1
> >>>>> Controller
> >>>>>    (rev 91)
> >>>>> 00:10.4 USB Controller: VIA Technologies, Inc. USB 2.0 (rev 90)
> >>>>> 00:11.0 ISA bridge: VIA Technologies, Inc. VT8251 PCI to ISA Bridge
> >>>>> 00:11.7 Host bridge: VIA Technologies, Inc. VT8251 Ultra VLINK Controller
> >>>>> 00:13.0 Host bridge: VIA Technologies, Inc. VT8251 Host Bridge
> >>>>> 00:13.1 PCI bridge: VIA Technologies, Inc. VT8251 PCI to PCI Bridge
> >>>>> 02:08.0 Ethernet controller: Broadcom Corporation BCM4401 100Base-T
> >>>>> (rev 02)
> >>>>> 02:09.0 Ethernet controller: Broadcom Corporation BCM4401 100Base-T
> >>>>> (rev 02)
> >>>>> 80:00.0 PCI bridge: VIA Technologies, Inc. VT8251 PCIE Root Port
> >>>>> 80:00.1 PCI bridge: VIA Technologies, Inc. VT8251 PCIE Root Port
> >>>>> 81:00.0 Ethernet controller: Broadcom Corporation NetLink BCM5906M
> >>>>> Fast Ethernet
> >>>>>    PCI Express (rev 02)
> >>>>> 82:00.0 Ethernet controller: Broadcom Corporation NetLink BCM5906M
> >>>>> Fast Ethernet
> >>>>>    PCI Express (rev 02)
> >>>>>
> >>>>> Kernel 2.6.36-2.el5.elrepo on an i686
> >>>>>
> >>>>> When I try to ifconfig either of the BCM5906M ports the system panics.
> >>>>>
> >>>>> Ideas, fixes ?
> >>>>>
> >>>>> [root@Z1010 ~]# modprobe tg3
> >>>>> [root@Z1010 ~]# ifconfig eth2 2.2.2.2/24
> >>>>> ------------[ cut here ]------------
> >>>>> kernel BUG at drivers/net/tg3.c:4365!
> >>>>> invalid opcode: 0000 [#1] PREEMPT SMP
> >>>>> last sysfs file: /sys/class/net/eth3/address
> >>>>> Modules linked in: tg3 xt_tcpudp ipt_LOG xt_limit xt_state
> >>>>> iptable_mangle af_ke]
> >>>>>
> >>>>> Pid: 20303, comm: kworker/0:2 Not tainted 2.6.36-2.el5.elrepo #1
> >>>>> CN700-8251/
> >>>>> EIP: 0060:[<e1c62f19>] EFLAGS: 00010202 CPU: 0
> >>>>> EIP is at tg3_tx_recover+0x1e/0x53 [tg3]
> >>>>> EAX: deece4c0 EBX: dfa9c000 ECX: deece4c0 EDX: ffffffff
> >>>>> ESI: deece4c0 EDI: deece500 EBP: c1801f38 ESP: c1801f30
> >>>>>    DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> >>>>> Process kworker/0:2 (pid: 20303, ti=c1801000 task=df0105d0
> >>>>> task.ti=dee62000)
> >>>>> Stack:
> >>>>>    dfa9c000 00000000 c1801f6c e1c630be c1801f6c deece4c0 00000840 00000000
> >>>>> <0>   df251cc0 00000005 00000000 df979800 deece500 deece4c0 00000040
> >>>>> c1801f94
> >>>>> <0>   e1c661e5 00000000 00000040 c1801f88 e09df1d2 00000000 deece500
> >>>>> dfab4000
> >>>>> Call Trace:
> >>>>>    [<e1c630be>] ? tg3_tx+0x157/0x1a2 [tg3]
> >>>>>    [<e1c661e5>] ? tg3_poll_work+0x2b/0x10b [tg3]
> >>>>>    [<e09df1d2>] ? ssb_write32+0x11/0x14 [b44]
> >>>>>    [<e1c662f9>] ? tg3_poll+0x34/0x9a [tg3]
> >>>>>    [<c0674058>] ? net_rx_action+0x7e/0x11c
> >>>>>    [<c04409c9>] ? __do_softirq+0x85/0x10c
> >>>>>    [<c0440944>] ? __do_softirq+0x0/0x10c
> >>>>> <IRQ>
> >>>>>    [<c04404ef>] ? _local_bh_enable_ip+0x68/0x87
> >>>>>    [<c044051b>] ? local_bh_enable_ip+0xd/0xf
> >>>>>    [<c046593b>] ? __raw_spin_unlock_bh+0x1c/0x1e
> >>>>>    [<c06fa4f2>] ? _raw_spin_unlock_bh+0xd/0xf
> >>>>>    [<e1c6281f>] ? spin_unlock_bh+0xd/0xf [tg3]
> >>>>>    [<e1c62cbe>] ? tg3_full_unlock+0x10/0x12 [tg3]
> >>>>>    [<e1c664c7>] ? tg3_reset_task+0xd7/0xe3 [tg3]
> >>>>>    [<c044ec37>] ? process_one_work+0x10b/0x1bc
> >>>>>    [<e1c663f0>] ? tg3_reset_task+0x0/0xe3 [tg3]
> >>>>>    [<c044fd41>] ? worker_thread+0x77/0xf9
> >>>>>    [<c0453048>] ? kthread+0x60/0x65
> >>>>>    [<c044fcca>] ? worker_thread+0x0/0xf9
> >>>>>    [<c0452fe8>] ? kthread+0x0/0x65
> >>>>>    [<c040337e>] ? kernel_thread_helper+0x6/0x10
> >>>>> Code: f0 e8 88 ff ff ff 8d 65 f8 5b 5e 5d c3 55 89 e5 56 53 0f 1f 44
> >>>>> 00 00 f6 8
> >>>>> EIP: [<e1c62f19>] tg3_tx_recover+0x1e/0x53 [tg3] SS:ESP 0068:c1801f30
> >>>>> ---[ end trace 82381e9b93e397ad ]---
> >>>>> Kernel panic - not syncing: Fatal exception in interrupt
> >>>>> Pid: 20303, comm: kworker/0:2 Tainted: G      D
> >>>>> 2.6.36-2.el5.elrepo #1
> >>>>> Call Trace:
> >>>>>    [<c043b3cd>] panic+0x62/0x15d
> >>>>>    [<c06fb7d1>] oops_end+0x99/0xa8
> >>>>>    [<e1c62f19>] ? tg3_tx_recover+0x1e/0x53 [tg3]
> >>>>>    [<c0405a62>] die+0x58/0x5e
> >>>>>
> >>>>> Thanks,
> >>>>> Steve
> >>>>>
> >>>>>
> >>>>>            
> >>>> Additonal info I compiled the latest kernel 2.6.37-rc8+ and still have the problem.
> >>>> Also boot with noapic I see this in the dmesg log and interrupts are increasing
> >>>> like crazy:
> >>>> tg3.c:v3.115 (October 14, 2010)
> >>>> tg3 0000:81:00.0: PCI INT A ->   Link[LNKA] ->   GSI 10 (level, low) ->   IRQ 10
> >>>> tg3 0000:81:00.0: setting latency timer to 64
> >>>> tg3 0000:81:00.0: PCI: Disallowing DAC for device
> >>>> tg3 0000:81:00.0: eth2: Tigon3 [partno(BCM95906) rev c002] (PCI Express) MAC add
> >>>> ress 00:02:b6:36:d1:39
> >>>> tg3 0000:81:00.0: eth2: attached PHY is 5906 (10/100Base-TX Ethernet) (WireSpeed
> >>>> [0])
> >>>> tg3 0000:81:00.0: eth2: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> >>>> tg3 0000:81:00.0: eth2: dma_rwctrl[76180000] dma_mask[32-bit]
> >>>> tg3 0000:82:00.0: PCI INT A ->   Link[LNKA] ->   GSI 10 (level, low) ->   IRQ 10
> >>>> tg3 0000:82:00.0: setting latency timer to 64
> >>>> tg3 0000:82:00.0: PCI: Disallowing DAC for device
> >>>> tg3 0000:82:00.0: eth3: Tigon3 [partno(BCM95906) rev c002] (PCI Express) MAC add
> >>>> ress 00:02:b6:36:d1:3a
> >>>> tg3 0000:82:00.0: eth3: attached PHY is 5906 (10/100Base-TX Ethernet) (WireSpeed
> >>>> [0])
> >>>> tg3 0000:82:00.0: eth3: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> >>>> tg3 0000:82:00.0: eth3: dma_rwctrl[76180000] dma_mask[32-bit]
> >>>> tg3 0000:81:00.0: irq 40 for MSI/MSI-X
> >>>> tg3 0000:81:00.0: eth2: No interrupt was generated using MSI. Switching to INTx
> >>>> mode. Please report this failure to the PCI maintainer and include system chipse
> >>>> t information
> >>>> ADDRCONF(NETDEV_UP): eth2: link is not ready
> >>>> [root@Z1010 ~]# cat /proc/interrupts
> >>>>               CPU0
> >>>>      0:        162    XT-PIC-XT-PIC    timer
> >>>>      1:          2    XT-PIC-XT-PIC    i8042
> >>>>      2:          0    XT-PIC-XT-PIC    cascade
> >>>>      3:          1    XT-PIC-XT-PIC
> >>>>      4:       4863    XT-PIC-XT-PIC    serial
> >>>>      6:          2    XT-PIC-XT-PIC    floppy
> >>>>      7:          5    XT-PIC-XT-PIC    ehci_hcd:usb1, uhci_hcd:usb3
> >>>>      8:          0    XT-PIC-XT-PIC    rtc0
> >>>>      9:          0    XT-PIC-XT-PIC    acpi
> >>>>     10:    2334234    XT-PIC-XT-PIC    uhci_hcd:usb2, eth0, eth2
> >>>>
> >>>> [root@Z1010 ~]# cat /proc/interrupts |grep eth2
> >>>>     10:   18388914    XT-PIC-XT-PIC    uhci_hcd:usb2, eth0, eth2
> >>>> [root@Z1010 ~]# cat /proc/interrupts |grep eth2
> >>>>     10:   18901627    XT-PIC-XT-PIC    uhci_hcd:usb2, eth0, eth2
> >>>>
> >>>> -- 
> >>>>
> >>>> "They that give up essential liberty to obtain temporary safety,
> >>>> deserve neither liberty nor safety."  (Ben Franklin)
> >>>>
> >>>> "The course of history shows that as a government grows, liberty
> >>>> decreases."  (Thomas Jefferson)
> >>>>
> >>>>          
> >>> I think drivers/net/tg3.c:4365 is at the line that reads
> >>> "spin_lock(&tp->lock);" in tg3_tx_recover.  Can you verify?
> >>>
> >>>
> >>>        
> >>
> >>           tg3_readphy(tp, MII_TG3_DSP_RW_PORT,&phy2);
> >>
> >> in static void tg3_serdes_parallel_detect(struct tg3 *tp)
> >>
> >> The driver version is:
> >> #define DRV_MODULE_NAME        "tg3"
> >> #define TG3_MAJ_NUM            3
> >> #define TG3_MIN_NUM            115
> >>      
> >
> > That doesn't look right.  The line number I quoted came from the kernel
> > panic output from 2.6.36-2.el5.elrepo.  I'm guessing you quoted me the
> > sources from the tg3.c file in 2.6.37-rc8+.  If you don't have the
> > 2.6.36-2.el5.elrepo sources readily available, can you give me the line
> > the kernel panic specifies from the tg3.c file from your 2.6.37-rc8+
> > sources?
> >
> >    
> Oops - You are correct. The problem is most of the time I don't get a 
> panic on the
> screen the box simply reboots.
> 
> I'll see if I can get the 2.6.36-2 sources - though they are suppose to 
> be the virgin
> kernel.org sources simply recompiled for Centos.
> 
> static void tg3_tx_recover(struct tg3 *tp)
> {
>      BUG_ON((tp->tg3_flags & TG3_FLAG_MBOX_WRITE_REORDER) ||
> 4365:           tp->write32_tx_mbox == tg3_write_indirect_mbox);
> 
> 
> > It looks like there are a lot of devices on IRQ 10.  Does the interrupt
> > count drop if you bring down eth0 (which I'm guessing is the b44 device)?
> >    
> This happens when I boot with noapic. Which I only did as a test. With 
> the noapic option
> the system doesn't panic - but gets all these extra interrupts as soon 
> as I ifconfig one of
> the 5906 ports.

I was wondering if the b44 device is having a problem with shared
interrupts.

> > Can you tell me if you saw the following message in the syslogs?
> >
> > "The system may be re-ordering memory-mapped I/O cycles to the network
> >   device, attempting to recover.  Please report the problem to the driver
> >   maintainer and include system chipset information."
> >
> >    
> Couldn't find this in the messages file.

Can you give me the output of 'lspci -vvv -xxx -s 81:00.0' and
'ethtool -i eth2'?

I'm wondering if this BUG_ON is a symptom of a different problem
masquerading as a write-reordering bug.  Do you have IPv6 configured?
If not, what happens if you just run 'ifconfig eth2 up', without
assigning an IP address?


^ permalink raw reply

* RE: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2011-01-12  3:30 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde,
	David Miller
In-Reply-To: <4D2C5845.4050805-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hi Wolfgang,

> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> Hi Bhupesh,
> 
> some final nitpicking as you need to fix Marc remarks anyway...
> 
> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
> > Bosch C_CAN controller is a full-CAN implementation which is
> compliant
> > to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can
> be
> > obtained from:
> > http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
> > c_can/users_manual_c_can.pdf
> >
> > This patch adds the support for this controller.
> > The following are the design choices made while writing the
> controller
> > driver:
> > 1. Interface Register set IF1 has be used only in the current design.
> > 2. Out of the 32 Message objects available, 16 are kept aside for RX
> >    purposes and the rest for TX purposes.
> > 3. NAPI implementation is such that both the TX and RX paths function
> >    in polling mode.
> >
> > Changes since V3:
> > 1. Corrected commenting style.
> > 2. Removing *non-required* header files from driver files.
> > 3. Removed extra *non-required* outer brackets globally.
> > 4. Implemented and used a inline routine to check BUSY status.
> > 5. Added a board-specific param *priv* inside the c_can_priv struct.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> 
> ...
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -0,0 +1,235 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * TX and RX NAPI implementation has been borrowed from at91 CAN
> driver
> > + * written by:
> > + * Copyright
> > + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> > + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef C_CAN_H
> > +#define C_CAN_H
> > +
> > +/* control register */
> > +#define CONTROL_TEST		BIT(7)
> > +#define CONTROL_CCE		BIT(6)
> > +#define CONTROL_DISABLE_AR	BIT(5)
> > +#define CONTROL_ENABLE_AR	(0 << 5)
> > +#define CONTROL_EIE		BIT(3)
> > +#define CONTROL_SIE		BIT(2)
> > +#define CONTROL_IE		BIT(1)
> > +#define CONTROL_INIT		BIT(0)
> > +
> > +/* test register */
> > +#define TEST_RX			BIT(7)
> > +#define TEST_TX1		BIT(6)
> > +#define TEST_TX2		BIT(5)
> > +#define TEST_LBACK		BIT(4)
> > +#define TEST_SILENT		BIT(3)
> > +#define TEST_BASIC		BIT(2)
> > +
> > +/* status register */
> > +#define STATUS_BOFF		BIT(7)
> > +#define STATUS_EWARN		BIT(6)
> > +#define STATUS_EPASS		BIT(5)
> > +#define STATUS_RXOK		BIT(4)
> > +#define STATUS_TXOK		BIT(3)
> > +#define STATUS_LEC_MASK		0x07
> > +
> > +/* error counter register */
> > +#define ERR_COUNTER_TEC_MASK	0xff
> > +#define ERR_COUNTER_TEC_SHIFT	0
> > +#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)
> 
> s/ERR_COUNTER/ERR_CNT/ to match the member name.

Ok.

> > +
> > +/* bit-timing register */
> > +#define BTR_BRP_MASK		0x3f
> > +#define BTR_BRP_SHIFT		0
> > +#define BTR_SJW_SHIFT		6
> > +#define BTR_SJW_MASK		(0x3 << BTR_SJW_SHIFT)
> > +#define BTR_TSEG1_SHIFT		8
> > +#define BTR_TSEG1_MASK		(0xf << BTR_TSEG1_SHIFT)
> > +#define BTR_TSEG2_SHIFT		12
> > +#define BTR_TSEG2_MASK		(0x7 << BTR_TSEG2_SHIFT)
> > +
> > +/* brp extension register */
> > +#define BRP_EXT_BRPE_MASK	0x0f
> > +#define BRP_EXT_BRPE_SHIFT	0
> > +
> > +/* IFx command request */
> > +#define IF_COMR_BUSY		BIT(15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR		BIT(7)
> > +#define IF_COMM_MASK		BIT(6)
> > +#define IF_COMM_ARB		BIT(5)
> > +#define IF_COMM_CONTROL		BIT(4)
> > +#define IF_COMM_CLR_INT_PND	BIT(3)
> > +#define IF_COMM_TXRQST		BIT(2)
> > +#define IF_COMM_DATAA		BIT(1)
> > +#define IF_COMM_DATAB		BIT(0)
> > +#define IF_COMM_ALL		(IF_COMM_MASK | IF_COMM_ARB | \
> > +				IF_COMM_CONTROL | IF_COMM_TXRQST | \
> > +				IF_COMM_DATAA | IF_COMM_DATAB)
> > +
> > +/* IFx arbitration */
> > +#define IF_ARB_MSGVAL		BIT(15)
> > +#define IF_ARB_MSGXTD		BIT(14)
> > +#define IF_ARB_TRANSMIT		BIT(13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT		BIT(15)
> > +#define IF_MCONT_MSGLST		BIT(14)
> > +#define IF_MCONT_CLR_MSGLST	(0 << 14)
> > +#define IF_MCONT_INTPND		BIT(13)
> > +#define IF_MCONT_UMASK		BIT(12)
> > +#define IF_MCONT_TXIE		BIT(11)
> > +#define IF_MCONT_RXIE		BIT(10)
> > +#define IF_MCONT_RMTEN		BIT(9)
> > +#define IF_MCONT_TXRQST		BIT(8)
> > +#define IF_MCONT_EOB		BIT(7)
> > +#define IF_MCONT_DLC_MASK	0xf
> > +
> > +/*
> > + * IFx register masks:
> > + * allow easy operation on 16-bit registers when the
> > + * argument is 32-bit instead
> > + */
> > +#define IFX_WRITE_LOW_16BIT(x)	((x) & 0xFFFF)
> > +#define IFX_WRITE_HIGH_16BIT(x)	(((x) & 0xFFFF0000) >> 16)
> > +
> > +/* message object split */
> > +#define C_CAN_NO_OF_OBJECTS	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_MSG_OBJ_RX_SPLIT	8
> > +#define C_CAN_MSG_RX_LOW_LAST	(C_CAN_MSG_OBJ_RX_SPLIT - 1)
> > +
> > +#define C_CAN_NEXT_MSG_OBJ_MASK	(C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS	0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT	0x8000
> > +
> > +/* global interrupt masks */
> > +#define ENABLE_ALL_INTERRUPTS	1
> > +#define DISABLE_ALL_INTERRUPTS	0
> > +
> > +/* minimum timeout for checking BUSY status */
> > +#define MIN_TIMEOUT_VALUE	6
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > +	u16 com_reg;
> > +	u16 com_mask;
> > +	u16 mask1;
> > +	u16 mask2;
> > +	u16 arb1;
> > +	u16 arb2;
> > +	u16 msg_cntrl;
> > +	u16 data[4];
> > +	u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > +	u16 control;
> > +	u16 status;
> > +	u16 err_cnt;
> > +	u16 btr;
> > +	u16 interrupt;
> > +	u16 test;
> > +	u16 brp_ext;
> > +	u16 _reserved1;
> > +	struct c_can_if_regs intf[2]; /* [0] = IF1 and [1] = IF2 */
> 
> I not happy with the name "intf". Sounds more like an interrupt related
> property. Above you already use the prefix IF_ for the corresponding
> macro definitions and somewhere else the name "iface".

I tried using the name *if* suggested by you here but the compiler will complain
considering it as a usage of if-construct. Do you have a better name that we
can use here?

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH] [RFC] ath9k: Fix reporting of RX STBC streams to userspace
From: Vasanthakumar Thiagarajan @ 2011-01-12  5:22 UTC (permalink / raw)
  To: Bernhard Walle
  Cc: Luis Rodriguez, Jouni Malinen, Vasanth Thiagarajan,
	Senthilkumar Balasubramanian, linville@tuxdriver.com,
	linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1294765194-30542-1-git-send-email-walle@corscience.de>

On Tue, Jan 11, 2011 at 10:29:54PM +0530, Bernhard Walle wrote:
> While the driver reports
> 
>         ath: TX streams 2, RX streams: 2
> 
> in the kernel log (with ATH_DBG_CONFIG set in the debug module
> parameter), "iw list" only reported
> 
> [...]
>         Capabilities: 0x12ce
>                 HT20/HT40
>                 SM Power Save disabled
>                 RX HT40 SGI
>                 TX STBC
>                 RX STBC 1-streams
> [...]
> 
> The driver seems to set the value as flag while the iw tool interprets
> it as number. This patch fixes that.

Nope. hw supports STBC for one Spatial stream only.

Vasanth

^ permalink raw reply

* Re: [PATCH v4 08/10] ARM: mxs: add ocotp read function
From: Shawn Guo @ 2011-01-12  6:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: davem, gerg, baruch, eric, bryan.wu, r64343, B32542,
	u.kleine-koenig, lw, w.sang, jamie, jamie, netdev,
	linux-arm-kernel
In-Reply-To: <20110111133137.GS12078@pengutronix.de>

Hi Sascha,

On Tue, Jan 11, 2011 at 02:31:37PM +0100, Sascha Hauer wrote:
> On Thu, Jan 06, 2011 at 03:13:16PM +0800, Shawn Guo wrote:
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Changes for v4:
> >  - Call cpu_relax() during polling
> > 
> > Changes for v2:
> >  - Add mutex locking for mxs_read_ocotp()
> >  - Use type size_t for count and i
> >  - Add comment for clk_enable/disable skipping
> >  - Add ERROR bit clearing and polling step
> > 
> >  arch/arm/mach-mxs/Makefile              |    2 +-
> >  arch/arm/mach-mxs/include/mach/common.h |    1 +
> >  arch/arm/mach-mxs/ocotp.c               |   79 +++++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/arm/mach-mxs/ocotp.c
> > 
> > diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> > index 39d3f9c..f23ebbd 100644
> > --- a/arch/arm/mach-mxs/Makefile
> > +++ b/arch/arm/mach-mxs/Makefile
> > @@ -1,5 +1,5 @@
> >  # Common support
> > -obj-y := clock.o devices.o gpio.o icoll.o iomux.o system.o timer.o
> > +obj-y := clock.o devices.o gpio.o icoll.o iomux.o ocotp.o system.o timer.o
> >  
> >  obj-$(CONFIG_SOC_IMX23) += clock-mx23.o mm-mx23.o
> >  obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
> > diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h
> > index 59133eb..cf02552 100644
> > --- a/arch/arm/mach-mxs/include/mach/common.h
> > +++ b/arch/arm/mach-mxs/include/mach/common.h
> > @@ -13,6 +13,7 @@
> >  
> >  struct clk;
> >  
> > +extern int mxs_read_ocotp(int offset, int count, u32 *values);
> >  extern int mxs_reset_block(void __iomem *);
> >  extern void mxs_timer_init(struct clk *, int);
> >  
> > diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
> > new file mode 100644
> > index 0000000..e2d39aa
> > --- /dev/null
> > +++ b/arch/arm/mach-mxs/ocotp.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright 2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +
> > +#include <mach/mxs.h>
> > +
> > +#define BM_OCOTP_CTRL_BUSY		(1 << 8)
> > +#define BM_OCOTP_CTRL_ERROR		(1 << 9)
> > +#define BM_OCOTP_CTRL_RD_BANK_OPEN	(1 << 12)
> > +
> > +static DEFINE_MUTEX(ocotp_mutex);
> > +
> > +int mxs_read_ocotp(unsigned offset, size_t count, u32 *values)
> > +{
> > +	void __iomem *ocotp_base = MXS_IO_ADDRESS(MXS_OCOTP_BASE_ADDR);
> > +	int timeout = 0x400;
> > +	size_t i;
> > +
> > +	mutex_lock(&ocotp_mutex);
> > +
> > +	/*
> > +	 * clk_enable(hbus_clk) for ocotp can be skipped
> > +	 * as it must be on when system is running.
> > +	 */
> > +
> > +	/* try to clear ERROR bit */
> > +	__mxs_clrl(BM_OCOTP_CTRL_ERROR, ocotp_base);
> 
> This operation does not try to clear the error bit but actually clears
> it...
> 
> > +
> > +	/* check both BUSY and ERROR cleared */
> > +	while ((__raw_readl(ocotp_base) &
> > +		(BM_OCOTP_CTRL_BUSY | BM_OCOTP_CTRL_ERROR)) && --timeout)
> > +		cpu_relax();
> 
> ...which means you do not have to poll the error bit here...
> 
> > +
> > +	if (unlikely(!timeout))
> > +		goto error_unlock;
> > +
> > +	/* open OCOTP banks for read */
> > +	__mxs_setl(BM_OCOTP_CTRL_RD_BANK_OPEN, ocotp_base);
> > +
> > +	/* approximately wait 32 hclk cycles */
> > +	udelay(1);
> > +
> > +	/* poll BUSY bit becoming cleared */
> > +	timeout = 0x400;
> > +	while ((__raw_readl(ocotp_base) & BM_OCOTP_CTRL_BUSY) && --timeout)
> > +		cpu_relax();
> 
> ...which means you can factor out a ocotp_wait_busy function and let the
> code speak instead of the comments.
> 
> > +
> > +	if (unlikely(!timeout))
> > +		goto error_unlock;
> > +
> > +	for (i = 0; i < count; i++, offset += 4)
> > +		*values++ = __raw_readl(ocotp_base + offset);
> 
> The registers in the ocotp are 16 byte aligned. Does it really make
> sense to provide a function allowing to read the gaps between the
> registers?
> 
Good catch.  The count was added to ease the consecutive otp word
reading, as there is bank open/close cost for otp read.  What about
the following changes?

int mxs_read_ocotp(unsigned offset, size_t otp_word_cnt, u32 *values)
{
	......

	for (i = 0; i < otp_word_cnt; i++, offset += 0x10)
		*values++ = __raw_readl(ocotp_base + offset);

	......
}


-- 
Regards,
Shawn


^ permalink raw reply

* [RFC] sched: CHOKe packet scheduler (v0.6)
From: Eric Dumazet @ 2011-01-12  7:13 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <1294727650.4148.20.camel@edumazet-laptop>

Hi Stephen, here is my v0.6 version :

- Added sanity checks before kcalloc()/kzalloc()
- Added a __GFP_NOWARN to kcalloc()
- Added call to qdisc_bstats_update() after commit bfe0d0298f2a67d94d5
(net_sched: factorize qdisc stats handling)

TODO :
- Added a stat specific update to track CHOKe probabilistic dual-drops
  I temporarily use requeues counter to make sure our code works

qdisc choke 11: parent 1:11 limit 70000b min 10000b max 30000b ewma 1 Plog 16 Scell_log 11
 Sent 236155665 bytes 429432 pkt (dropped 1644435, overlimits 1251933 requeues 196251) 
 rate 38800Kbit 8820pps backlog 124438905b 30001p requeues 196251 
  marked 0 early 1251933 pdrop 0 other 0


Thanks !

diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index e69de29..1c62292 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -0,0 +1,540 @@
+/*
+ * net/sched/sch_choke.c	CHOKE scheduler
+ *
+ * Copyright (c) 2011 Stephen Hemminger <shemminger@vyatta.com>
+ * Copyright (c) 2011 Eric Dumazet <eric.dumazet@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/reciprocal_div.h>
+#include <net/pkt_sched.h>
+#include <net/inet_ecn.h>
+#include <net/red.h>
+
+/*	CHOKe stateless AQM for fair bandwidth allocation
+        =================================================
+
+   CHOKe (CHOose and Keep for responsive flows, CHOose and Kill for
+   unresponsive flows) is a variant of RED that penalizes misbehaving flows but
+   maintains no flow state. The difference from RED is an additional step
+   during the enqueuing process. If average queue size is over the
+   low threshold (qmin), a packet is chosen at random from the queue.
+   If both the new and chosen packet are from the same flow, both
+   are dropped. Unlike RED, CHOKe is not really a "classful" qdisc because it
+   needs to access packets in queue randomly. It has a minimal class
+   interface to allow overriding the builtin flow classifier with
+   filters.
+
+   Source:
+   R. Pan, B. Prabhakar, and K. Psounis, "CHOKe, A Stateless
+   Active Queue Management Scheme for Approximating Fair Bandwidth Allocation",
+   IEEE INFOCOM, 2000.
+
+   A. Tang, J. Wang, S. Low, "Understanding CHOKe: Throughput and Spatial
+   Characteristics", IEEE/ACM Transactions on Networking, 2004
+
+ */
+
+struct choke_sched_data {
+/* Parameters */
+	u32		 limit;
+	unsigned char	 flags;
+
+	struct red_parms parms;
+	struct red_stats stats;
+
+/* Variables */
+	struct tcf_proto *filter_list;
+	unsigned int	 head;
+	unsigned int	 tail;
+	unsigned int	 holes;
+	unsigned int	 tab_mask; /* size - 1 */
+
+	struct sk_buff **tab;
+};
+
+static inline unsigned int choke_len(const struct choke_sched_data *q)
+{
+	return (q->tail - q->head) & q->tab_mask;
+}
+
+/* deliver a random number between 0 and N - 1 */
+static inline u32 random_N(unsigned int N)
+{
+	return reciprocal_divide(random32(), N);
+}
+
+
+/* Select a packet at random from the queue in O(1) and handle holes */
+static struct sk_buff *choke_peek_random(struct choke_sched_data *q,
+					 unsigned int *pidx)
+{
+	struct sk_buff *skb;
+	int retrys = 3;
+
+	do {
+		*pidx = (q->head + random_N(choke_len(q))) & q->tab_mask;
+		skb = q->tab[*pidx];
+		if (skb)
+			return skb;
+	} while (--retrys > 0);
+
+	/* queue is has lots of holes use the head which is known to exist */
+	return q->tab[*pidx = q->head];
+}
+
+/* Is ECN parameter configured */
+static inline int use_ecn(const struct choke_sched_data *q)
+{
+	return q->flags & TC_RED_ECN;
+}
+
+/* Should packets over max just be dropped (versus marked) */
+static inline int use_harddrop(const struct choke_sched_data *q)
+{
+	return q->flags & TC_RED_HARDDROP;
+}
+
+/* Move head pointer forward to skip over holes */
+static void choke_zap_head_holes(struct choke_sched_data *q)
+{
+	while (q->holes && q->tab[q->head] == NULL) {
+		q->head = (q->head + 1) & q->tab_mask;
+		q->holes--;
+	}
+}
+
+/* Move tail pointer backwards to reuse holes */
+static void choke_zap_tail_holes(struct choke_sched_data *q)
+{
+	while (q->holes && q->tab[q->tail - 1] == NULL) {
+		q->tail = (q->tail - 1) & q->tab_mask;
+		q->holes--;
+	}
+}
+
+/* Drop packet from queue array by creating a "hole" */
+static void choke_drop_by_idx(struct choke_sched_data *q, unsigned int idx)
+{
+	q->tab[idx] = NULL;
+	q->holes++;
+
+	if (idx == q->head)
+		choke_zap_head_holes(q);
+	if (idx == q->tail)
+		choke_zap_tail_holes(q);
+}
+
+/* Classify flow using either:
+   1. pre-existing classification result in skb
+   2. fast internal classification
+   3. use TC filter based classification
+*/
+static inline unsigned int choke_classify(struct sk_buff *skb,
+					  struct Qdisc *sch, int *qerr)
+
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct tcf_result res;
+	int result;
+
+	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
+
+	if (TC_H_MAJ(skb->priority) == sch->handle &&
+	    TC_H_MIN(skb->priority) > 0)
+		return TC_H_MIN(skb->priority);
+
+	if (!q->filter_list)
+		return skb_get_rxhash(skb);
+
+	result = tc_classify(skb, q->filter_list, &res);
+	if (result >= 0) {
+#ifdef CONFIG_NET_CLS_ACT
+		switch (result) {
+		case TC_ACT_STOLEN:
+		case TC_ACT_QUEUED:
+			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+		case TC_ACT_SHOT:
+			return 0;
+		}
+#endif
+		return TC_H_MIN(res.classid);
+	}
+
+	return 0;
+}
+
+static int choke_enqueue(struct sk_buff *skb, struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct red_parms *p = &q->parms;
+	unsigned int hash;
+	int uninitialized_var(ret);
+
+	hash = choke_classify(skb, sch, &ret);
+	if (unlikely(!hash)) {
+		if (ret & __NET_XMIT_BYPASS)
+			sch->qstats.drops++;
+		kfree_skb(skb);
+		return ret;
+	}
+
+	/* XXX add hash to qdisc_skb_cb? */
+	*(unsigned int *)(qdisc_skb_cb(skb)->data) = hash;
+
+	/* Compute average queue usage (see RED) */
+	p->qavg = red_calc_qavg(p, choke_len(q) - q->holes);
+	if (red_is_idling(p))
+		red_end_of_idle_period(p);
+
+	/* Is queue small? */
+	if (p->qavg <= p->qth_min)
+		p->qcount = -1;
+	else {
+		struct sk_buff *oskb;
+		unsigned int idx;
+
+		/* Draw a packet at random from queue */
+		oskb = choke_peek_random(q, &idx);
+
+		/* Both packets from same flow ? */
+		if (*(unsigned int *)(qdisc_skb_cb(oskb)->data) == hash) {
+			/* Drop both packets */
+			choke_drop_by_idx(q, idx);
+			qdisc_drop(oskb, sch);
+			sch->qstats.requeues++;
+			goto congestion_drop;
+		}
+
+		if (p->qavg > p->qth_max) {
+			p->qcount = -1;
+
+			sch->qstats.overlimits++;
+			if (use_harddrop(q) || !use_ecn(q) ||
+			    !INET_ECN_set_ce(skb)) {
+				q->stats.forced_drop++;
+				goto congestion_drop;
+			}
+
+			q->stats.forced_mark++;
+		}
+
+		if (++p->qcount) {
+			if (red_mark_probability(p, p->qavg)) {
+				p->qcount = 0;
+				p->qR = red_random(p);
+
+				sch->qstats.overlimits++;
+				if (!use_ecn(q) || !INET_ECN_set_ce(skb)) {
+					q->stats.prob_drop++;
+					goto congestion_drop;
+				}
+
+				q->stats.prob_mark++;
+			}
+		} else
+			p->qR = red_random(p);
+	}
+
+	/* Admit new packet */
+	if (likely(choke_len(q) < q->limit)) {
+		q->tab[q->tail] = skb;
+		q->tail = (q->tail + 1) & q->tab_mask;
+
+		sch->qstats.backlog += qdisc_pkt_len(skb);
+		qdisc_bstats_update(sch, skb);
+		sch->q.qlen = choke_len(q) - q->holes;
+		return NET_XMIT_SUCCESS;
+	}
+
+	q->stats.pdrop++;
+	sch->qstats.drops++;
+	kfree_skb(skb);
+	return NET_XMIT_DROP;
+
+ congestion_drop:
+	qdisc_drop(skb, sch);
+	return NET_XMIT_CN;
+}
+
+static struct sk_buff *choke_dequeue(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct sk_buff *skb;
+
+	if (q->head == q->tail) {
+		if (!red_is_idling(&q->parms))
+			red_start_of_idle_period(&q->parms);
+		return NULL;
+	}
+	skb = q->tab[q->head];
+	q->tab[q->head] = NULL; /* not really needed */
+	q->head = (q->head + 1) & q->tab_mask;
+	choke_zap_head_holes(q);
+	sch->qstats.backlog -= qdisc_pkt_len(skb);
+	sch->q.qlen = choke_len(q) - q->holes;
+
+	return skb;
+}
+
+static unsigned int choke_drop(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	unsigned int len;
+
+	len = qdisc_queue_drop(sch);
+
+	if (len > 0)
+		q->stats.other++;
+	else {
+		if (!red_is_idling(&q->parms))
+			red_start_of_idle_period(&q->parms);
+	}
+
+	return len;
+}
+
+static void choke_reset(struct Qdisc* sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	red_restart(&q->parms);
+}
+
+static const struct nla_policy choke_policy[TCA_RED_MAX + 1] = {
+	[TCA_RED_PARMS]	= { .len = sizeof(struct tc_red_qopt) },
+	[TCA_RED_STAB]	= { .len = RED_STAB_SIZE },
+};
+
+
+static void choke_free(void *addr)
+{
+	if (addr) {
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
+	}
+}
+
+static int choke_change(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct nlattr *tb[TCA_RED_MAX + 1];
+	struct tc_red_qopt *ctl;
+	int err;
+	struct sk_buff **old = NULL;
+	unsigned int mask;
+
+	if (opt == NULL)
+		return -EINVAL;
+
+	err = nla_parse_nested(tb, TCA_RED_MAX, opt, choke_policy);
+	if (err < 0)
+		return err;
+
+	if (tb[TCA_RED_PARMS] == NULL ||
+	    tb[TCA_RED_STAB] == NULL)
+		return -EINVAL;
+
+	ctl = nla_data(tb[TCA_RED_PARMS]);
+
+	mask = roundup_pow_of_two(ctl->limit + 1) - 1;
+	/* limit queue size to one MB */
+	if (mask + 1 > (1U << 20) / sizeof(struct sk_buff *))
+		return -EINVAL;
+	if (mask != q->tab_mask) {
+		struct sk_buff **ntab = kcalloc(mask + 1, sizeof(struct sk_buff *),
+						GFP_KERNEL | __GFP_NOWARN);
+		if (!ntab)
+			ntab = vzalloc((mask + 1) * sizeof(struct sk_buff *));
+		if (!ntab)
+			return -ENOMEM;
+		sch_tree_lock(sch);
+		old = q->tab;
+		if (old) {
+			unsigned int tail = 0;
+
+			while (q->head != q->tail) {
+				ntab[tail++] = q->tab[q->head];
+				q->head = (q->head + 1) & q->tab_mask;
+			}
+			q->head = 0;
+			q->tail = tail;
+		}
+		q->tab_mask = mask;
+		q->tab = ntab;
+		q->holes = 0;
+	} else
+		sch_tree_lock(sch);
+	q->flags = ctl->flags;
+	q->limit = ctl->limit;
+
+	red_set_parms(&q->parms, ctl->qth_min, ctl->qth_max, ctl->Wlog,
+		      ctl->Plog, ctl->Scell_log,
+		      nla_data(tb[TCA_RED_STAB]));
+
+	if (q->head == q->tail)
+		red_end_of_idle_period(&q->parms);
+
+	sch_tree_unlock(sch);
+	choke_free(old);
+	return 0;
+}
+
+static int choke_init(struct Qdisc* sch, struct nlattr *opt)
+{
+	return choke_change(sch, opt);
+}
+
+static int choke_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct nlattr *opts = NULL;
+	struct tc_red_qopt opt = {
+		.limit		= q->limit,
+		.flags		= q->flags,
+		.qth_min	= q->parms.qth_min >> q->parms.Wlog,
+		.qth_max	= q->parms.qth_max >> q->parms.Wlog,
+		.Wlog		= q->parms.Wlog,
+		.Plog		= q->parms.Plog,
+		.Scell_log	= q->parms.Scell_log,
+	};
+
+	opts = nla_nest_start(skb, TCA_OPTIONS);
+	if (opts == NULL)
+		goto nla_put_failure;
+
+	NLA_PUT(skb, TCA_RED_PARMS, sizeof(opt), &opt);
+	return nla_nest_end(skb, opts);
+
+nla_put_failure:
+	nla_nest_cancel(skb, opts);
+	return -EMSGSIZE;
+}
+
+static int choke_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+	struct tc_red_xstats st = {
+		.early	= q->stats.prob_drop + q->stats.forced_drop,
+		.pdrop	= q->stats.pdrop,
+		.other	= q->stats.other,
+		.marked	= q->stats.prob_mark + q->stats.forced_mark,
+	};
+
+	return gnet_stats_copy_app(d, &st, sizeof(st));
+}
+
+static void choke_destroy(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	tcf_destroy_chain(&q->filter_list);
+	choke_free(q->tab);
+}
+
+static struct Qdisc *choke_leaf(struct Qdisc *sch, unsigned long arg)
+{
+	return NULL;
+}
+
+static unsigned long choke_get(struct Qdisc *sch, u32 classid)
+{
+	return 0;
+}
+
+static void choke_put(struct Qdisc *q, unsigned long cl)
+{
+}
+
+static unsigned long choke_bind(struct Qdisc *sch, unsigned long parent,
+				u32 classid)
+{
+	return 0;
+}
+
+static struct tcf_proto **choke_find_tcf(struct Qdisc *sch, unsigned long cl)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	if (cl)
+		return NULL;
+	return &q->filter_list;
+}
+
+static int choke_dump_class(struct Qdisc *sch, unsigned long cl,
+			  struct sk_buff *skb, struct tcmsg *tcm)
+{
+	tcm->tcm_handle |= TC_H_MIN(cl);
+	return 0;
+}
+
+static void choke_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+	if (!arg->stop) {
+		if (arg->fn(sch, 1, arg) < 0) {
+			arg->stop = 1;
+			return;
+		}
+		arg->count++;
+	}
+}
+
+static const struct Qdisc_class_ops choke_class_ops = {
+	.leaf		=	choke_leaf,
+	.get		=	choke_get,
+	.put		=	choke_put,
+	.tcf_chain	=	choke_find_tcf,
+	.bind_tcf	=	choke_bind,
+	.unbind_tcf	=	choke_put,
+	.dump		=	choke_dump_class,
+	.walk		=	choke_walk,
+};
+
+static struct sk_buff *choke_peek_head(struct Qdisc *sch)
+{
+	struct choke_sched_data *q = qdisc_priv(sch);
+
+	return (q->head != q->tail) ? q->tab[q->head] : NULL;
+}
+
+static struct Qdisc_ops choke_qdisc_ops __read_mostly = {
+	.id		=	"choke",
+	.priv_size	=	sizeof(struct choke_sched_data),
+
+	.enqueue	=	choke_enqueue,
+	.dequeue	=	choke_dequeue,
+	.peek		=	choke_peek_head,
+	.drop		=	choke_drop,
+	.init		=	choke_init,
+	.destroy	=	choke_destroy,
+	.reset		=	choke_reset,
+	.change		=	choke_change,
+	.dump		=	choke_dump,
+	.dump_stats	=	choke_dump_stats,
+	.owner		=	THIS_MODULE,
+};
+
+static int __init choke_module_init(void)
+{
+	return register_qdisc(&choke_qdisc_ops);
+}
+
+static void __exit choke_module_exit(void)
+{
+	unregister_qdisc(&choke_qdisc_ops);
+}
+
+module_init(choke_module_init)
+module_exit(choke_module_exit)
+
+MODULE_LICENSE("GPL");



^ permalink raw reply related

* Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro
From: Richard Cochran @ 2011-01-12  7:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	Alan Cox, Arnd Bergmann, Christoph Lameter, David Miller,
	John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti
In-Reply-To: <alpine.LFD.2.00.1101111224280.12146-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>

On Tue, Jan 11, 2011 at 01:57:23PM +0100, Thomas Gleixner wrote:
> I wonder whether we should be a bit more clever here and create an
> extra entry for posix_cpu_timers in the posix_clocks array and do the
> following:
...
> That reduces the code significantly and the MAX_CLOCKS check in
> clock_get_array_id() replaces the invalid_clock() check in the
> syscalls as well. It does not matter whether we check this before or
> after copying stuff from user.

Well, this does reduce the number of LOC, but the number of
comparisons is the same. I think the code size would be also no
different.

> Adding your new stuff requires just another entry in the array, the
> setup of the function pointers and the CLOCKFD check in
> clock_get_array_id(). Everything else just falls in place.

For me, I am not sure if either version is really very pretty or easy
to understand.

My instinct is to keep the posix_cpu_X and pc_X functions out of the
array of static clock id functions, if only to make the distinction
between the legacy static ids and new dynamic ids clear.

The conclusion from the "dynamic clock as file" discussion was that we
don't want to add any more static clock ids, and new clocks should use
the new, dynamic clock API. So, we should discourage any future
attempt to add to that function array!

Having said that, if you insist on it, I won't mind reworking the
dispatch code as you suggested.

> > +
> > +#define CLOCK_DISPATCH(clock, call, arglist) dispatch_##call arglist
> > +
> 
> Can we get rid of this completely please ?

Yes, gladly.

> >  clock_nanosleep_restart(struct restart_block *restart_block)
> >  {
> > -	clockid_t which_clock = restart_block->arg0;
> > -
> 
> How does that compile ?

Because the CLOCK_DISPATCH macro, above, makes no use of the first
argument! I have removed the macro for the next round.

> >  	return CLOCK_DISPATCH(which_clock, nsleep_restart,
> >  			      (restart_block));
> >  }

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH v4 05/10] net/fec: add dual fec support for mx28
From: Uwe Kleine-König @ 2011-01-12  7:42 UTC (permalink / raw)
  To: Greg Ungerer
  Cc: baruch, B32542, netdev, Sascha Hauer, jamie, w.sang, r64343,
	linux-arm-kernel, eric, bryan.wu, jamie, davem, Shawn Guo, lw
In-Reply-To: <4D2C5A55.7050502@snapgear.com>

Hi Greg,

On Tue, Jan 11, 2011 at 11:25:41PM +1000, Greg Ungerer wrote:
> On 11/01/11 23:07, Uwe Kleine-König wrote:
> >On Tue, Jan 11, 2011 at 10:24:12PM +1000, Greg Ungerer wrote:
> >>On 11/01/11 20:27, Sascha Hauer wrote:
> >>>On Thu, Jan 06, 2011 at 03:13:13PM +0800, Shawn Guo wrote:
> >>>This option is used nowhere and should be removed. Certainly it does not
> >>>have the effect of enabling the second ethernet controller.
> >>
> >>It does for a ColdFire platform...
> >>
> >>grep -r CONFIG_FEC2 *
> >>
> >>arch/m68knommu/configs/m5275evb_defconfig:CONFIG_FEC2=y
> >>arch/m68knommu/platform/527x/config.c:#ifdef CONFIG_FEC2
> >>arch/m68knommu/platform/527x/config.c:#ifdef CONFIG_FEC2
> >IMHO Sascha's comment[1] applies here, too.
> 
> I am not arguing that it doesn't :-)
> 
> Simply that removing that config option and doing nothing
> else would not be the right thing to do.
> 
> Regards
> Greg
> 
> 
> >And someone might want to do what he suggested soon or the patch
> >removing CONFIG_FEC2[2] needs to be commented accordingly.
note that davem took the patch removing CONFIG_FEC2 now.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH 10/16] drivers/net: stmmac: don't treat NULL clk as an error
From: Peppe CAVALLARO @ 2011-01-12  8:00 UTC (permalink / raw)
  To: Jamie Iles; +Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1294749833-32019-11-git-send-email-jamie@jamieiles.com>

Hello

On 1/11/2011 1:43 PM, Jamie Iles wrote:
>
> clk_get() returns a struct clk cookie to the driver and some platforms
> may return NULL if they only support a single clock.  clk_get() has only
> failed if it returns a ERR_PTR() encoded pointer.
>
Thanks for the patch that actually useful.
I'm going to rework this part of the driver.
At any rate, all my new changes can be done on top of this patch.

Regards,
Peppe
>
>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  drivers/net/stmmac/stmmac_timer.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/stmmac/stmmac_timer.c
> b/drivers/net/stmmac/stmmac_timer.c
> index 2a0e1ab..ebb1b37 100644
> --- a/drivers/net/stmmac/stmmac_timer.c
> +++ b/drivers/net/stmmac/stmmac_timer.c
> @@ -91,6 +91,7 @@ int stmmac_close_ext_timer(void)
>
>  #elif defined(CONFIG_STMMAC_TMU_TIMER)
>  #include <linux/clk.h>
> +#include <linux/err.h>
>  #define TMU_CHANNEL "tmu2_clk"
>  static struct clk *timer_clock;
>
> @@ -109,7 +110,7 @@ int stmmac_open_ext_timer(struct net_device *dev,
> struct stmmac_timer *tm)
>  {
>         timer_clock = clk_get(NULL, TMU_CHANNEL);
>
> -       if (timer_clock == NULL)
> +       if (IS_ERR(timer_clock))
>                 return -1;
>
>         if (tmu2_register_user(stmmac_timer_handler, (void *)dev) < 0) {
> --
> 1.7.3.4
>

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2011-01-12  8:37 UTC (permalink / raw)
  To: Bhupesh SHARMA
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde,
	David Miller
In-Reply-To: <D5ECB3C7A6F99444980976A8C6D896384DEAFA3066-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>

On 01/12/2011 04:30 AM, Bhupesh SHARMA wrote:
> Hi Wolfgang,
> 
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>> Hi Bhupesh,
>>
>> some final nitpicking as you need to fix Marc remarks anyway...
>>
>> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
>>> Bosch C_CAN controller is a full-CAN implementation which is
>> compliant
>>> to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can
>> be
>>> obtained from:
>>> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
>>> c_can/users_manual_c_can.pdf
>>>
>>> This patch adds the support for this controller.
>>> The following are the design choices made while writing the
>> controller
>>> driver:
>>> 1. Interface Register set IF1 has be used only in the current design.
>>> 2. Out of the 32 Message objects available, 16 are kept aside for RX
>>>    purposes and the rest for TX purposes.
>>> 3. NAPI implementation is such that both the TX and RX paths function
>>>    in polling mode.
>>>
>>> Changes since V3:
>>> 1. Corrected commenting style.
>>> 2. Removing *non-required* header files from driver files.
>>> 3. Removed extra *non-required* outer brackets globally.
>>> 4. Implemented and used a inline routine to check BUSY status.
>>> 5. Added a board-specific param *priv* inside the c_can_priv struct.
>>>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>>
>> ...
>>> +++ b/drivers/net/can/c_can/c_can.h
>>> @@ -0,0 +1,235 @@
>>> +/*
>>> + * CAN bus driver for Bosch C_CAN controller
>>> + *
>>> + * Copyright (C) 2010 ST Microelectronics
>>> + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>>> + *
>>> + * Borrowed heavily from the C_CAN driver originally written by:
>>> + * Copyright (C) 2007
>>> + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
>> <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
>>> + *
>>> + * TX and RX NAPI implementation has been borrowed from at91 CAN
>> driver
>>> + * written by:
>>> + * Copyright
>>> + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>> + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> + *
>>> + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
>> part A and B.
>>> + * Bosch C_CAN user manual can be obtained from:
>>> + *
>> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
>>> + * users_manual_c_can.pdf
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2. This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#ifndef C_CAN_H
>>> +#define C_CAN_H
>>> +
>>> +/* control register */
>>> +#define CONTROL_TEST		BIT(7)
>>> +#define CONTROL_CCE		BIT(6)
>>> +#define CONTROL_DISABLE_AR	BIT(5)
>>> +#define CONTROL_ENABLE_AR	(0 << 5)
>>> +#define CONTROL_EIE		BIT(3)
>>> +#define CONTROL_SIE		BIT(2)
>>> +#define CONTROL_IE		BIT(1)
>>> +#define CONTROL_INIT		BIT(0)
>>> +
>>> +/* test register */
>>> +#define TEST_RX			BIT(7)
>>> +#define TEST_TX1		BIT(6)
>>> +#define TEST_TX2		BIT(5)
>>> +#define TEST_LBACK		BIT(4)
>>> +#define TEST_SILENT		BIT(3)
>>> +#define TEST_BASIC		BIT(2)
>>> +
>>> +/* status register */
>>> +#define STATUS_BOFF		BIT(7)
>>> +#define STATUS_EWARN		BIT(6)
>>> +#define STATUS_EPASS		BIT(5)
>>> +#define STATUS_RXOK		BIT(4)
>>> +#define STATUS_TXOK		BIT(3)
>>> +#define STATUS_LEC_MASK		0x07
>>> +
>>> +/* error counter register */
>>> +#define ERR_COUNTER_TEC_MASK	0xff
>>> +#define ERR_COUNTER_TEC_SHIFT	0
>>> +#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)
>>
>> s/ERR_COUNTER/ERR_CNT/ to match the member name.
> 
> Ok.
> 
>>> +
>>> +/* bit-timing register */
>>> +#define BTR_BRP_MASK		0x3f
>>> +#define BTR_BRP_SHIFT		0
>>> +#define BTR_SJW_SHIFT		6
>>> +#define BTR_SJW_MASK		(0x3 << BTR_SJW_SHIFT)
>>> +#define BTR_TSEG1_SHIFT		8
>>> +#define BTR_TSEG1_MASK		(0xf << BTR_TSEG1_SHIFT)
>>> +#define BTR_TSEG2_SHIFT		12
>>> +#define BTR_TSEG2_MASK		(0x7 << BTR_TSEG2_SHIFT)
>>> +
>>> +/* brp extension register */
>>> +#define BRP_EXT_BRPE_MASK	0x0f
>>> +#define BRP_EXT_BRPE_SHIFT	0
>>> +
>>> +/* IFx command request */
>>> +#define IF_COMR_BUSY		BIT(15)
>>> +
>>> +/* IFx command mask */
>>> +#define IF_COMM_WR		BIT(7)
>>> +#define IF_COMM_MASK		BIT(6)
>>> +#define IF_COMM_ARB		BIT(5)
>>> +#define IF_COMM_CONTROL		BIT(4)
>>> +#define IF_COMM_CLR_INT_PND	BIT(3)
>>> +#define IF_COMM_TXRQST		BIT(2)
>>> +#define IF_COMM_DATAA		BIT(1)
>>> +#define IF_COMM_DATAB		BIT(0)
>>> +#define IF_COMM_ALL		(IF_COMM_MASK | IF_COMM_ARB | \
>>> +				IF_COMM_CONTROL | IF_COMM_TXRQST | \
>>> +				IF_COMM_DATAA | IF_COMM_DATAB)
>>> +
>>> +/* IFx arbitration */
>>> +#define IF_ARB_MSGVAL		BIT(15)
>>> +#define IF_ARB_MSGXTD		BIT(14)
>>> +#define IF_ARB_TRANSMIT		BIT(13)
>>> +
>>> +/* IFx message control */
>>> +#define IF_MCONT_NEWDAT		BIT(15)
>>> +#define IF_MCONT_MSGLST		BIT(14)
>>> +#define IF_MCONT_CLR_MSGLST	(0 << 14)
>>> +#define IF_MCONT_INTPND		BIT(13)
>>> +#define IF_MCONT_UMASK		BIT(12)
>>> +#define IF_MCONT_TXIE		BIT(11)
>>> +#define IF_MCONT_RXIE		BIT(10)
>>> +#define IF_MCONT_RMTEN		BIT(9)
>>> +#define IF_MCONT_TXRQST		BIT(8)
>>> +#define IF_MCONT_EOB		BIT(7)
>>> +#define IF_MCONT_DLC_MASK	0xf
>>> +
>>> +/*
>>> + * IFx register masks:
>>> + * allow easy operation on 16-bit registers when the
>>> + * argument is 32-bit instead
>>> + */
>>> +#define IFX_WRITE_LOW_16BIT(x)	((x) & 0xFFFF)
>>> +#define IFX_WRITE_HIGH_16BIT(x)	(((x) & 0xFFFF0000) >> 16)
>>> +
>>> +/* message object split */
>>> +#define C_CAN_NO_OF_OBJECTS	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_MSG_OBJ_RX_SPLIT	8
>>> +#define C_CAN_MSG_RX_LOW_LAST	(C_CAN_MSG_OBJ_RX_SPLIT - 1)
>>> +
>>> +#define C_CAN_NEXT_MSG_OBJ_MASK	(C_CAN_MSG_OBJ_TX_NUM - 1)
>>> +#define RECEIVE_OBJECT_BITS	0x0000ffff
>>> +
>>> +/* status interrupt */
>>> +#define STATUS_INTERRUPT	0x8000
>>> +
>>> +/* global interrupt masks */
>>> +#define ENABLE_ALL_INTERRUPTS	1
>>> +#define DISABLE_ALL_INTERRUPTS	0
>>> +
>>> +/* minimum timeout for checking BUSY status */
>>> +#define MIN_TIMEOUT_VALUE	6
>>> +
>>> +/* napi related */
>>> +#define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
>>> +
>>> +/* c_can IF registers */
>>> +struct c_can_if_regs {
>>> +	u16 com_reg;
>>> +	u16 com_mask;
>>> +	u16 mask1;
>>> +	u16 mask2;
>>> +	u16 arb1;
>>> +	u16 arb2;
>>> +	u16 msg_cntrl;
>>> +	u16 data[4];
>>> +	u16 _reserved[13];
>>> +};
>>> +
>>> +/* c_can hardware registers */
>>> +struct c_can_regs {
>>> +	u16 control;
>>> +	u16 status;
>>> +	u16 err_cnt;
>>> +	u16 btr;
>>> +	u16 interrupt;
>>> +	u16 test;
>>> +	u16 brp_ext;
>>> +	u16 _reserved1;
>>> +	struct c_can_if_regs intf[2]; /* [0] = IF1 and [1] = IF2 */
>>
>> I not happy with the name "intf". Sounds more like an interrupt related
>> property. Above you already use the prefix IF_ for the corresponding
>> macro definitions and somewhere else the name "iface".
> 
> I tried using the name *if* suggested by you here but the compiler will complain
> considering it as a usage of if-construct. Do you have a better name that we
> can use here?

Oh, I was not aware ot that. Sorry for the noise! Then your old "ifregs"
or "iface" would be fine, I think. I just see that the pch_can uses
"ifregs" as well.

Wolfgang.

^ permalink raw reply

* RE: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2011-01-12  8:38 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde,
	David Miller
In-Reply-To: <4D2D6830.9090701-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hi Wolfgang,

> On 01/12/2011 04:30 AM, Bhupesh SHARMA wrote:
> > Hi Wolfgang,
> >
> >> -----Original Message-----
> >> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >> Hi Bhupesh,
> >>
> >> some final nitpicking as you need to fix Marc remarks anyway...
> >>
> >> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
> >>> Bosch C_CAN controller is a full-CAN implementation which is
> >> compliant
> >>> to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual
> can
> >> be
> >>> obtained from:
> >>> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
> >>> c_can/users_manual_c_can.pdf
> >>>
> >>> This patch adds the support for this controller.
> >>> The following are the design choices made while writing the
> >> controller
> >>> driver:
> >>> 1. Interface Register set IF1 has be used only in the current
> design.
> >>> 2. Out of the 32 Message objects available, 16 are kept aside for
> RX
> >>>    purposes and the rest for TX purposes.
> >>> 3. NAPI implementation is such that both the TX and RX paths
> function
> >>>    in polling mode.
> >>>
> >>> Changes since V3:
> >>> 1. Corrected commenting style.
> >>> 2. Removing *non-required* header files from driver files.
> >>> 3. Removed extra *non-required* outer brackets globally.
> >>> 4. Implemented and used a inline routine to check BUSY status.
> >>> 5. Added a board-specific param *priv* inside the c_can_priv
> struct.
> >>>
> >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> >>
> >> ...
> >>> +++ b/drivers/net/can/c_can/c_can.h
> >>> @@ -0,0 +1,235 @@
> >>> +/*
> >>> + * CAN bus driver for Bosch C_CAN controller
> >>> + *
> >>> + * Copyright (C) 2010 ST Microelectronics
> >>> + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> >>> + *
> >>> + * Borrowed heavily from the C_CAN driver originally written by:
> >>> + * Copyright (C) 2007
> >>> + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> >> <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>> + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> >>> + *
> >>> + * TX and RX NAPI implementation has been borrowed from at91 CAN
> >> driver
> >>> + * written by:
> >>> + * Copyright
> >>> + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> >>> + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>> + *
> >>> + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> >> part A and B.
> >>> + * Bosch C_CAN user manual can be obtained from:
> >>> + *
> >> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> >>> + * users_manual_c_can.pdf
> >>> + *
> >>> + * This file is licensed under the terms of the GNU General Public
> >>> + * License version 2. This program is licensed "as is" without any
> >>> + * warranty of any kind, whether express or implied.
> >>> + */
> >>> +
> >>> +#ifndef C_CAN_H
> >>> +#define C_CAN_H
> >>> +
> >>> +/* control register */
> >>> +#define CONTROL_TEST		BIT(7)
> >>> +#define CONTROL_CCE		BIT(6)
> >>> +#define CONTROL_DISABLE_AR	BIT(5)
> >>> +#define CONTROL_ENABLE_AR	(0 << 5)
> >>> +#define CONTROL_EIE		BIT(3)
> >>> +#define CONTROL_SIE		BIT(2)
> >>> +#define CONTROL_IE		BIT(1)
> >>> +#define CONTROL_INIT		BIT(0)
> >>> +
> >>> +/* test register */
> >>> +#define TEST_RX			BIT(7)
> >>> +#define TEST_TX1		BIT(6)
> >>> +#define TEST_TX2		BIT(5)
> >>> +#define TEST_LBACK		BIT(4)
> >>> +#define TEST_SILENT		BIT(3)
> >>> +#define TEST_BASIC		BIT(2)
> >>> +
> >>> +/* status register */
> >>> +#define STATUS_BOFF		BIT(7)
> >>> +#define STATUS_EWARN		BIT(6)
> >>> +#define STATUS_EPASS		BIT(5)
> >>> +#define STATUS_RXOK		BIT(4)
> >>> +#define STATUS_TXOK		BIT(3)
> >>> +#define STATUS_LEC_MASK		0x07
> >>> +
> >>> +/* error counter register */
> >>> +#define ERR_COUNTER_TEC_MASK	0xff
> >>> +#define ERR_COUNTER_TEC_SHIFT	0
> >>> +#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)
> >>
> >> s/ERR_COUNTER/ERR_CNT/ to match the member name.
> >
> > Ok.
> >
> >>> +
> >>> +/* bit-timing register */
> >>> +#define BTR_BRP_MASK		0x3f
> >>> +#define BTR_BRP_SHIFT		0
> >>> +#define BTR_SJW_SHIFT		6
> >>> +#define BTR_SJW_MASK		(0x3 << BTR_SJW_SHIFT)
> >>> +#define BTR_TSEG1_SHIFT		8
> >>> +#define BTR_TSEG1_MASK		(0xf << BTR_TSEG1_SHIFT)
> >>> +#define BTR_TSEG2_SHIFT		12
> >>> +#define BTR_TSEG2_MASK		(0x7 << BTR_TSEG2_SHIFT)
> >>> +
> >>> +/* brp extension register */
> >>> +#define BRP_EXT_BRPE_MASK	0x0f
> >>> +#define BRP_EXT_BRPE_SHIFT	0
> >>> +
> >>> +/* IFx command request */
> >>> +#define IF_COMR_BUSY		BIT(15)
> >>> +
> >>> +/* IFx command mask */
> >>> +#define IF_COMM_WR		BIT(7)
> >>> +#define IF_COMM_MASK		BIT(6)
> >>> +#define IF_COMM_ARB		BIT(5)
> >>> +#define IF_COMM_CONTROL		BIT(4)
> >>> +#define IF_COMM_CLR_INT_PND	BIT(3)
> >>> +#define IF_COMM_TXRQST		BIT(2)
> >>> +#define IF_COMM_DATAA		BIT(1)
> >>> +#define IF_COMM_DATAB		BIT(0)
> >>> +#define IF_COMM_ALL		(IF_COMM_MASK | IF_COMM_ARB | \
> >>> +				IF_COMM_CONTROL | IF_COMM_TXRQST | \
> >>> +				IF_COMM_DATAA | IF_COMM_DATAB)
> >>> +
> >>> +/* IFx arbitration */
> >>> +#define IF_ARB_MSGVAL		BIT(15)
> >>> +#define IF_ARB_MSGXTD		BIT(14)
> >>> +#define IF_ARB_TRANSMIT		BIT(13)
> >>> +
> >>> +/* IFx message control */
> >>> +#define IF_MCONT_NEWDAT		BIT(15)
> >>> +#define IF_MCONT_MSGLST		BIT(14)
> >>> +#define IF_MCONT_CLR_MSGLST	(0 << 14)
> >>> +#define IF_MCONT_INTPND		BIT(13)
> >>> +#define IF_MCONT_UMASK		BIT(12)
> >>> +#define IF_MCONT_TXIE		BIT(11)
> >>> +#define IF_MCONT_RXIE		BIT(10)
> >>> +#define IF_MCONT_RMTEN		BIT(9)
> >>> +#define IF_MCONT_TXRQST		BIT(8)
> >>> +#define IF_MCONT_EOB		BIT(7)
> >>> +#define IF_MCONT_DLC_MASK	0xf
> >>> +
> >>> +/*
> >>> + * IFx register masks:
> >>> + * allow easy operation on 16-bit registers when the
> >>> + * argument is 32-bit instead
> >>> + */
> >>> +#define IFX_WRITE_LOW_16BIT(x)	((x) & 0xFFFF)
> >>> +#define IFX_WRITE_HIGH_16BIT(x)	(((x) & 0xFFFF0000) >> 16)
> >>> +
> >>> +/* message object split */
> >>> +#define C_CAN_NO_OF_OBJECTS	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_MSG_OBJ_RX_SPLIT	8
> >>> +#define C_CAN_MSG_RX_LOW_LAST	(C_CAN_MSG_OBJ_RX_SPLIT - 1)
> >>> +
> >>> +#define C_CAN_NEXT_MSG_OBJ_MASK	(C_CAN_MSG_OBJ_TX_NUM - 1)
> >>> +#define RECEIVE_OBJECT_BITS	0x0000ffff
> >>> +
> >>> +/* status interrupt */
> >>> +#define STATUS_INTERRUPT	0x8000
> >>> +
> >>> +/* global interrupt masks */
> >>> +#define ENABLE_ALL_INTERRUPTS	1
> >>> +#define DISABLE_ALL_INTERRUPTS	0
> >>> +
> >>> +/* minimum timeout for checking BUSY status */
> >>> +#define MIN_TIMEOUT_VALUE	6
> >>> +
> >>> +/* napi related */
> >>> +#define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
> >>> +
> >>> +/* c_can IF registers */
> >>> +struct c_can_if_regs {
> >>> +	u16 com_reg;
> >>> +	u16 com_mask;
> >>> +	u16 mask1;
> >>> +	u16 mask2;
> >>> +	u16 arb1;
> >>> +	u16 arb2;
> >>> +	u16 msg_cntrl;
> >>> +	u16 data[4];
> >>> +	u16 _reserved[13];
> >>> +};
> >>> +
> >>> +/* c_can hardware registers */
> >>> +struct c_can_regs {
> >>> +	u16 control;
> >>> +	u16 status;
> >>> +	u16 err_cnt;
> >>> +	u16 btr;
> >>> +	u16 interrupt;
> >>> +	u16 test;
> >>> +	u16 brp_ext;
> >>> +	u16 _reserved1;
> >>> +	struct c_can_if_regs intf[2]; /* [0] = IF1 and [1] = IF2 */
> >>
> >> I not happy with the name "intf". Sounds more like an interrupt
> related
> >> property. Above you already use the prefix IF_ for the corresponding
> >> macro definitions and somewhere else the name "iface".
> >
> > I tried using the name *if* suggested by you here but the compiler
> will complain
> > considering it as a usage of if-construct. Do you have a better name
> that we
> > can use here?
> 
> Oh, I was not aware ot that. Sorry for the noise! Then your old
> "ifregs"
> or "iface" would be fine, I think. I just see that the pch_can uses
> "ifregs" as well.
> 

Yes, I get checked the pch_can driver as well. It also uses the name *ifregs*.
Let's keep the name *ifregs* then.

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2011-01-12  8:41 UTC (permalink / raw)
  To: Bhupesh SHARMA
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde,
	David Miller
In-Reply-To: <D5ECB3C7A6F99444980976A8C6D896384DEAFA31AD-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>

On 01/12/2011 09:38 AM, Bhupesh SHARMA wrote:
> Hi Wolfgang,
> 
>> On 01/12/2011 04:30 AM, Bhupesh SHARMA wrote:
>>> Hi Wolfgang,
>>>
>>>> -----Original Message-----
>>>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>> Hi Bhupesh,
>>>>
>>>> some final nitpicking as you need to fix Marc remarks anyway...
>>>>
>>>> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
>>>>> Bosch C_CAN controller is a full-CAN implementation which is
>>>> compliant
>>>>> to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual
>> can
>>>> be
>>>>> obtained from:
>>>>> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
>>>>> c_can/users_manual_c_can.pdf
>>>>>
>>>>> This patch adds the support for this controller.
>>>>> The following are the design choices made while writing the
>>>> controller
>>>>> driver:
>>>>> 1. Interface Register set IF1 has be used only in the current
>> design.
>>>>> 2. Out of the 32 Message objects available, 16 are kept aside for
>> RX
>>>>>    purposes and the rest for TX purposes.
>>>>> 3. NAPI implementation is such that both the TX and RX paths
>> function
>>>>>    in polling mode.
>>>>>
>>>>> Changes since V3:
>>>>> 1. Corrected commenting style.
>>>>> 2. Removing *non-required* header files from driver files.
>>>>> 3. Removed extra *non-required* outer brackets globally.
>>>>> 4. Implemented and used a inline routine to check BUSY status.
>>>>> 5. Added a board-specific param *priv* inside the c_can_priv
>> struct.
>>>>>
>>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>>>>
>>>> ...
>>>>> +++ b/drivers/net/can/c_can/c_can.h
>>>>> @@ -0,0 +1,235 @@
>>>>> +/*
>>>>> + * CAN bus driver for Bosch C_CAN controller
>>>>> + *
>>>>> + * Copyright (C) 2010 ST Microelectronics
>>>>> + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>>>>> + *
>>>>> + * Borrowed heavily from the C_CAN driver originally written by:
>>>>> + * Copyright (C) 2007
>>>>> + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
>>>> <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>>> + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
>>>>> + *
>>>>> + * TX and RX NAPI implementation has been borrowed from at91 CAN
>>>> driver
>>>>> + * written by:
>>>>> + * Copyright
>>>>> + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>>>> + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>>> + *
>>>>> + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
>>>> part A and B.
>>>>> + * Bosch C_CAN user manual can be obtained from:
>>>>> + *
>>>> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
>>>>> + * users_manual_c_can.pdf
>>>>> + *
>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>> + * License version 2. This program is licensed "as is" without any
>>>>> + * warranty of any kind, whether express or implied.
>>>>> + */
>>>>> +
>>>>> +#ifndef C_CAN_H
>>>>> +#define C_CAN_H
>>>>> +
>>>>> +/* control register */
>>>>> +#define CONTROL_TEST		BIT(7)
>>>>> +#define CONTROL_CCE		BIT(6)
>>>>> +#define CONTROL_DISABLE_AR	BIT(5)
>>>>> +#define CONTROL_ENABLE_AR	(0 << 5)
>>>>> +#define CONTROL_EIE		BIT(3)
>>>>> +#define CONTROL_SIE		BIT(2)
>>>>> +#define CONTROL_IE		BIT(1)
>>>>> +#define CONTROL_INIT		BIT(0)
>>>>> +
>>>>> +/* test register */
>>>>> +#define TEST_RX			BIT(7)
>>>>> +#define TEST_TX1		BIT(6)
>>>>> +#define TEST_TX2		BIT(5)
>>>>> +#define TEST_LBACK		BIT(4)
>>>>> +#define TEST_SILENT		BIT(3)
>>>>> +#define TEST_BASIC		BIT(2)
>>>>> +
>>>>> +/* status register */
>>>>> +#define STATUS_BOFF		BIT(7)
>>>>> +#define STATUS_EWARN		BIT(6)
>>>>> +#define STATUS_EPASS		BIT(5)
>>>>> +#define STATUS_RXOK		BIT(4)
>>>>> +#define STATUS_TXOK		BIT(3)
>>>>> +#define STATUS_LEC_MASK		0x07
>>>>> +
>>>>> +/* error counter register */
>>>>> +#define ERR_COUNTER_TEC_MASK	0xff
>>>>> +#define ERR_COUNTER_TEC_SHIFT	0
>>>>> +#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)
>>>>
>>>> s/ERR_COUNTER/ERR_CNT/ to match the member name.
>>>
>>> Ok.
>>>
>>>>> +
>>>>> +/* bit-timing register */
>>>>> +#define BTR_BRP_MASK		0x3f
>>>>> +#define BTR_BRP_SHIFT		0
>>>>> +#define BTR_SJW_SHIFT		6
>>>>> +#define BTR_SJW_MASK		(0x3 << BTR_SJW_SHIFT)
>>>>> +#define BTR_TSEG1_SHIFT		8
>>>>> +#define BTR_TSEG1_MASK		(0xf << BTR_TSEG1_SHIFT)
>>>>> +#define BTR_TSEG2_SHIFT		12
>>>>> +#define BTR_TSEG2_MASK		(0x7 << BTR_TSEG2_SHIFT)
>>>>> +
>>>>> +/* brp extension register */
>>>>> +#define BRP_EXT_BRPE_MASK	0x0f
>>>>> +#define BRP_EXT_BRPE_SHIFT	0
>>>>> +
>>>>> +/* IFx command request */
>>>>> +#define IF_COMR_BUSY		BIT(15)
>>>>> +
>>>>> +/* IFx command mask */
>>>>> +#define IF_COMM_WR		BIT(7)
>>>>> +#define IF_COMM_MASK		BIT(6)
>>>>> +#define IF_COMM_ARB		BIT(5)
>>>>> +#define IF_COMM_CONTROL		BIT(4)
>>>>> +#define IF_COMM_CLR_INT_PND	BIT(3)
>>>>> +#define IF_COMM_TXRQST		BIT(2)
>>>>> +#define IF_COMM_DATAA		BIT(1)
>>>>> +#define IF_COMM_DATAB		BIT(0)
>>>>> +#define IF_COMM_ALL		(IF_COMM_MASK | IF_COMM_ARB | \
>>>>> +				IF_COMM_CONTROL | IF_COMM_TXRQST | \
>>>>> +				IF_COMM_DATAA | IF_COMM_DATAB)
>>>>> +
>>>>> +/* IFx arbitration */
>>>>> +#define IF_ARB_MSGVAL		BIT(15)
>>>>> +#define IF_ARB_MSGXTD		BIT(14)
>>>>> +#define IF_ARB_TRANSMIT		BIT(13)
>>>>> +
>>>>> +/* IFx message control */
>>>>> +#define IF_MCONT_NEWDAT		BIT(15)
>>>>> +#define IF_MCONT_MSGLST		BIT(14)
>>>>> +#define IF_MCONT_CLR_MSGLST	(0 << 14)
>>>>> +#define IF_MCONT_INTPND		BIT(13)
>>>>> +#define IF_MCONT_UMASK		BIT(12)
>>>>> +#define IF_MCONT_TXIE		BIT(11)
>>>>> +#define IF_MCONT_RXIE		BIT(10)
>>>>> +#define IF_MCONT_RMTEN		BIT(9)
>>>>> +#define IF_MCONT_TXRQST		BIT(8)
>>>>> +#define IF_MCONT_EOB		BIT(7)
>>>>> +#define IF_MCONT_DLC_MASK	0xf
>>>>> +
>>>>> +/*
>>>>> + * IFx register masks:
>>>>> + * allow easy operation on 16-bit registers when the
>>>>> + * argument is 32-bit instead
>>>>> + */
>>>>> +#define IFX_WRITE_LOW_16BIT(x)	((x) & 0xFFFF)
>>>>> +#define IFX_WRITE_HIGH_16BIT(x)	(((x) & 0xFFFF0000) >> 16)
>>>>> +
>>>>> +/* message object split */
>>>>> +#define C_CAN_NO_OF_OBJECTS	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_MSG_OBJ_RX_SPLIT	8
>>>>> +#define C_CAN_MSG_RX_LOW_LAST	(C_CAN_MSG_OBJ_RX_SPLIT - 1)
>>>>> +
>>>>> +#define C_CAN_NEXT_MSG_OBJ_MASK	(C_CAN_MSG_OBJ_TX_NUM - 1)
>>>>> +#define RECEIVE_OBJECT_BITS	0x0000ffff
>>>>> +
>>>>> +/* status interrupt */
>>>>> +#define STATUS_INTERRUPT	0x8000
>>>>> +
>>>>> +/* global interrupt masks */
>>>>> +#define ENABLE_ALL_INTERRUPTS	1
>>>>> +#define DISABLE_ALL_INTERRUPTS	0
>>>>> +
>>>>> +/* minimum timeout for checking BUSY status */
>>>>> +#define MIN_TIMEOUT_VALUE	6
>>>>> +
>>>>> +/* napi related */
>>>>> +#define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
>>>>> +
>>>>> +/* c_can IF registers */
>>>>> +struct c_can_if_regs {
>>>>> +	u16 com_reg;
>>>>> +	u16 com_mask;
>>>>> +	u16 mask1;
>>>>> +	u16 mask2;
>>>>> +	u16 arb1;
>>>>> +	u16 arb2;
>>>>> +	u16 msg_cntrl;
>>>>> +	u16 data[4];
>>>>> +	u16 _reserved[13];
>>>>> +};
>>>>> +
>>>>> +/* c_can hardware registers */
>>>>> +struct c_can_regs {
>>>>> +	u16 control;
>>>>> +	u16 status;
>>>>> +	u16 err_cnt;
>>>>> +	u16 btr;
>>>>> +	u16 interrupt;
>>>>> +	u16 test;
>>>>> +	u16 brp_ext;
>>>>> +	u16 _reserved1;
>>>>> +	struct c_can_if_regs intf[2]; /* [0] = IF1 and [1] = IF2 */
>>>>
>>>> I not happy with the name "intf". Sounds more like an interrupt
>> related
>>>> property. Above you already use the prefix IF_ for the corresponding
>>>> macro definitions and somewhere else the name "iface".
>>>
>>> I tried using the name *if* suggested by you here but the compiler
>> will complain
>>> considering it as a usage of if-construct. Do you have a better name
>> that we
>>> can use here?
>>
>> Oh, I was not aware ot that. Sorry for the noise! Then your old
>> "ifregs"
>> or "iface" would be fine, I think. I just see that the pch_can uses
>> "ifregs" as well.
>>
> 
> Yes, I get checked the pch_can driver as well. It also uses the name *ifregs*.
> Let's keep the name *ifregs* then.

D'accord.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2011-01-12  8:47 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
	Wolfram Sang, Oliver Hartkopp
In-Reply-To: <4D2CD34B.8000609-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On 01/11/2011 11:01 PM, Marc Kleine-Budde wrote:
> On 01/11/2011 07:25 PM, Wolfgang Grandegger wrote:
> 
> [...]
> 
>> I was told that Bosch's page for the C_CAN is here:
>>
>>  http://www.semiconductors.bosch.de/en/ipmodules/can/canipmodules/c_can/c_can.asp
>>
>> There it's also written for what bus interfaces the C_CAN is available for, e.g.:
>>
>>   "The ASIC version is delivered with the AMBA APB bus interface from ARM."
> 
> On ARM we also have the AMBA bus abstraction in Linux, but I've never
> worked with it. IIRC Wolfram (Cc'ed) has recently touched it.
> 
>> which is obviously used in the "Platform Controller Hub" eg20t.
> 
> But in the pch, they've attached the AMBA to the PCI bus.

Ah, right. Anyway, with that driver we are prepared to handle other bus
interfaces by adding another bus-sensitive driver in
"drivers/net/can/c_can".

Wolfgang.

^ permalink raw reply

* RE: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Bhupesh SHARMA @ 2011-01-12  8:51 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
	Wolfgang Grandegger
In-Reply-To: <4D2C4D68.3070705-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Hi Marc,

Thanks for your review.
Please see my comments inline:

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
> > Bosch C_CAN controller is a full-CAN implementation which is
> compliant
> > to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can
> > be obtained from:
> > http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/
> > c_can/users_manual_c_can.pdf
> >
> > This patch adds the support for this controller.
> > The following are the design choices made while writing the
> controller
> > driver:
> > 1. Interface Register set IF1 has be used only in the current design.
> > 2. Out of the 32 Message objects available, 16 are kept aside for RX
> >    purposes and the rest for TX purposes.
> > 3. NAPI implementation is such that both the TX and RX paths function
> >    in polling mode.
>
> Can you replace all "dev_<LEVEL>(dev->dev.parent," by
> "netdev_<LEVEL>(dev,".

ACK.

> >
> > Changes since V3:
> > 1. Corrected commenting style.
> > 2. Removing *non-required* header files from driver files.
> > 3. Removed extra *non-required* outer brackets globally.
> > 4. Implemented and used a inline routine to check BUSY status.
> > 5. Added a board-specific param *priv* inside the c_can_priv struct.
>
> The changes are usually placed between the "---" and the diffstat, so
> that they don't show up in the commit message in the git repo.

Ok.

> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > ---
> >  drivers/net/can/Kconfig                |    2 +
> >  drivers/net/can/Makefile               |    1 +
> >  drivers/net/can/c_can/Kconfig          |   15 +
> >  drivers/net/can/c_can/Makefile         |    8 +
> >  drivers/net/can/c_can/c_can.c          |  953
> ++++++++++++++++++++++++++++++++
> >  drivers/net/can/c_can/c_can.h          |  235 ++++++++
> >  drivers/net/can/c_can/c_can_platform.c |  208 +++++++
> >  7 files changed, 1422 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/net/can/c_can/Kconfig  create mode 100644
> > drivers/net/can/c_can/Makefile  create mode 100644
> > drivers/net/can/c_can/c_can.c  create mode 100644
> > drivers/net/can/c_can/c_can.h  create mode 100644
> > drivers/net/can/c_can/c_can_platform.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > 9d9e453..50549b5 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -86,6 +86,8 @@ source "drivers/net/can/mscan/Kconfig"
> >
> >  source "drivers/net/can/sja1000/Kconfig"
> >
> > +source "drivers/net/can/c_can/Kconfig"
> > +
> >  source "drivers/net/can/usb/Kconfig"
> >
> >  config CAN_DEBUG_DEVICES
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index
> > 0057537..c3efeb3 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -11,6 +11,7 @@ obj-y                             += usb/
> >
> >  obj-$(CONFIG_CAN_SJA1000)  += sja1000/
> >  obj-$(CONFIG_CAN_MSCAN)            += mscan/
> > +obj-$(CONFIG_CAN_C_CAN)            += c_can/
> >  obj-$(CONFIG_CAN_AT91)             += at91_can.o
> >  obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
> >  obj-$(CONFIG_CAN_MCP251X)  += mcp251x.o
> > diff --git a/drivers/net/can/c_can/Kconfig
> > b/drivers/net/can/c_can/Kconfig new file mode 100644 index
> > 0000000..ffb9773
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Kconfig
> > @@ -0,0 +1,15 @@
> > +menuconfig CAN_C_CAN
> > +   tristate "Bosch C_CAN devices"
> > +   depends on CAN_DEV && HAS_IOMEM
> > +
> > +if CAN_C_CAN
> > +
> > +config CAN_C_CAN_PLATFORM
> > +   tristate "Generic Platform Bus based C_CAN driver"
> > +   ---help---
> > +     This driver adds support for the C_CAN chips connected to
> > +     the "platform bus" (Linux abstraction for directly to the
> > +     processor attached devices) which can be found on various
> > +     boards from ST Microelectronics (http://www.st.com)
> > +     like the SPEAr1310 and SPEAr320 evaluation boards.
> > +endif
> > diff --git a/drivers/net/can/c_can/Makefile
> > b/drivers/net/can/c_can/Makefile new file mode 100644 index
> > 0000000..9273f6d
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/Makefile
> > @@ -0,0 +1,8 @@
> > +#
> > +#  Makefile for the Bosch C_CAN controller drivers.
> > +#
> > +
> > +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> > +obj-$(CONFIG_CAN_C_CAN_PLATFORM) += c_can_platform.o
> > +
> > +ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> > diff --git a/drivers/net/can/c_can/c_can.c
> > b/drivers/net/can/c_can/c_can.c new file mode 100644 index
> > 0000000..06e1553
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -0,0 +1,953 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * TX and RX NAPI implementation has been borrowed from at91 CAN
> > +driver
> > + * written by:
>
> I've just cleaned up the RX implementation, have a look at [1] and [2].

I am not sure that C_CAN will be effected as per the at91 driver changes.
In C_CAN, messages numbers are allowed only from 0x01 to 0x20. Message object
number 0 is not allowed. Internally obj-put and obj-get routines increment the
message number by 1.

> > + * Copyright
> > + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>                                ^^^^^^^^^^^^^^^^^^^
>
> Hans has recently changed his email address, it's "hjk-vqZO0P4V72/QD6PfKP4TzA@public.gmane.org"

Ok :)

> > + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/if_arp.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/list.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +
> > +#include <linux/can.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +
> > +#include "c_can.h"
> > +
> > +static struct can_bittiming_const c_can_bittiming_const = {
> > +   .name = KBUILD_MODNAME,
> > +   .tseg1_min = 2,         /* Time segment 1 = prop_seg + phase_seg1
> */
> > +   .tseg1_max = 16,
> > +   .tseg2_min = 1,         /* Time segment 2 = phase_seg2 */
> > +   .tseg2_max = 8,
> > +   .sjw_max = 4,
> > +   .brp_min = 1,
> > +   .brp_max = 1024,        /* 6-bit BRP field + 4-bit BRPE field*/
> > +   .brp_inc = 1,
> > +};
> > +
> > +static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
> > +{
> > +   return (priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) +
> > +                   C_CAN_MSG_OBJ_TX_FIRST;
> > +}
> > +
> > +static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg) {
> > +   u32 val = priv->read_reg(priv, reg);
> > +   val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> > +   return val;
> > +}
> > +
> > +static void c_can_enable_all_interrupts(struct c_can_priv *priv,
> > +                                           int enable)
> > +{
> > +   unsigned int cntrl_save = priv->read_reg(priv,
> > +                                           &priv->regs->control);
> > +
> > +   if (enable)
> > +           cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> > +   else
> > +           cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> > +
> > +   priv->write_reg(priv, &priv->regs->control, cntrl_save); }
> > +
> > +static inline int c_can_check_busy_status(struct c_can_priv *priv,
> > +int iface) {
> > +   int count = MIN_TIMEOUT_VALUE;
>
> I'd insert a blank line, here.

Ok.

> > +   while (count && priv->read_reg(priv,
> > +                           &priv->regs->intf[iface].com_reg) &
> > +                           IF_COMR_BUSY) {
> > +           count--;
> > +           udelay(1);
> > +   }
> > +
> > +   return count;
> > +}
> > +
> > +static inline void c_can_object_get(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].com_mask,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->intf[iface].com_reg,
> > +                   IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           dev_err(dev->dev.parent, "timed out in object get\n"); }
> > +
> > +static inline void c_can_object_put(struct net_device *dev,
> > +                                   int iface, int objno, int mask)
> > +{
> > +   int ret;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /*
> > +    * As per specs, after writting the message object number in the
> > +    * IF command request register the transfer b/w interface
> > +    * register and message RAM must be complete in 6 CAN-CLK
> > +    * period.
> > +    */
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].com_mask,
> > +                   (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > +   priv->write_reg(priv, &priv->regs->intf[iface].com_reg,
> > +                   IFX_WRITE_LOW_16BIT(objno + 1));
> > +
> > +   ret = c_can_check_busy_status(priv, iface);
> > +   if (!ret)
> > +           dev_err(dev->dev.parent, "timed out in object put\n"); }
> > +
> > +int c_can_write_msg_object(struct net_device *dev,
> > +                   int iface, struct can_frame *frame, int objno) {
> > +   int i;
> > +   u16 flags = 0;
> > +   unsigned int id;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (!(frame->can_id & CAN_RTR_FLAG))
> > +           flags |= IF_ARB_TRANSMIT;
> > +
> > +   if (frame->can_id & CAN_EFF_FLAG) {
> > +           id = frame->can_id & CAN_EFF_MASK;
> > +           flags |= IF_ARB_MSGXTD;
> > +   } else
> > +           id = ((frame->can_id & CAN_SFF_MASK) << 18);
> > +
> > +   flags |= IF_ARB_MSGVAL;
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb1,
> > +                           IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb2, flags |
> > +                           IFX_WRITE_HIGH_16BIT(id));
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           priv->write_reg(priv, &priv->regs->intf[iface].data[i / 2],
> > +                           frame->data[i] | (frame->data[i + 1] << 8));
> > +   }
> > +
> > +   return frame->can_dlc;
> > +}
> > +
> > +static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> > +
> > +}
> > +
> > +static inline void c_can_activate_all_lower_rx_msg_obj(struct
> net_device *dev,
> > +                                           int iface,
> > +                                           int ctrl_mask)
> > +{
> > +   int i;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   for (i = 0; i < C_CAN_MSG_RX_LOW_LAST; i++) {
> > +           priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl,
> > +                           ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                                   IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +           c_can_object_put(dev, iface, i + 1, IF_COMM_CONTROL);
> > +   }
> > +}
> > +
> > +static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
> > +                                           int iface, int ctrl_mask,
> > +                                           int obj)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl,
> > +                   ctrl_mask & ~(IF_MCONT_MSGLST |
> > +                           IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> > +   c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> > +
> > +}
> > +
> > +static void c_can_handle_lost_msg_obj(struct net_device *dev,
> > +                                   int iface, int objno)
> > +{
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   dev_err(dev->dev.parent, "msg lost in buffer %d\n", objno);
> > +
> > +   c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> > +                                           ~IF_COMM_TXRQST);
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl,
> > +                   IF_MCONT_CLR_MSGLST);
> > +
> > +   c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
> > +
> > +   /* create an error msg */
> > +   skb = alloc_can_err_skb(dev, &frame);
> > +   if (unlikely(!skb))
> > +           return;
> > +
> > +   frame->can_id |= CAN_ERR_CRTL;
> > +   frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> > +   stats->rx_errors++;
> > +   stats->rx_over_errors++;
> > +
> > +   netif_receive_skb(skb);
> > +}
> > +
> > +static int c_can_read_msg_object(struct net_device *dev, int iface,
> int ctrl,
> > +                           int objno)
> > +{
> > +   u16 flags, data;
> > +   int i;
> > +   unsigned int val;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   struct net_device_stats *stats = &dev->stats;
> > +   struct sk_buff *skb;
> > +   struct can_frame *frame;
> > +
> > +   skb = alloc_can_skb(dev, &frame);
> > +   if (!skb) {
> > +           stats->rx_dropped++;
> > +           return -ENOMEM;
> > +   }
> > +
> > +   frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> > +
> > +   for (i = 0; i < frame->can_dlc; i += 2) {
> > +           data = priv->read_reg(priv,
> > +                           &priv->regs->intf[iface].data[i / 2]);
> > +           frame->data[i] = data;
> > +           frame->data[i + 1] = data >> 8;
> > +   }
> > +
> > +   flags = priv->read_reg(priv, &priv->regs->intf[iface].arb2);
> > +   val = priv->read_reg(priv, &priv->regs->intf[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;
> > +
> > +   netif_receive_skb(skb);
> > +
> > +   stats->rx_packets++;
> > +   stats->rx_bytes += frame->can_dlc;
> > +
> > +   return 0;
> > +}
> > +
> > +static void c_can_setup_receive_object(struct net_device *dev, int
> iface,
> > +                                   int objno, unsigned int mask,
> > +                                   unsigned int id, unsigned int mcont) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].mask1,
> > +                   IFX_WRITE_LOW_16BIT(mask));
> > +   priv->write_reg(priv, &priv->regs->intf[iface].mask2,
> > +                   IFX_WRITE_HIGH_16BIT(mask));
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb1,
> > +                   IFX_WRITE_LOW_16BIT(id));
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb2,
> > +                   (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl, mcont);
> > +   c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> ~IF_COMM_TXRQST);
> > +   dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1));
> > +
> > +}
> > +
> > +static void c_can_inval_msg_object(struct net_device *dev, int
> iface,
> > +int objno) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb1, 0);
> > +   priv->write_reg(priv, &priv->regs->intf[iface].arb2, 0);
> > +   priv->write_reg(priv, &priv->regs->intf[iface].msg_cntrl, 0);
> > +
> > +   c_can_object_put(dev, iface, objno,
> > +                           IF_COMM_ARB | IF_COMM_CONTROL);
> > +   dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> > +                   c_can_read_reg32(priv, &priv->regs->msgval1));
> > +
> > +}
> > +
> > +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> > +                                   struct net_device *dev)
> > +{
> > +   u32 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->regs->intf[0].msg_cntrl,
> > +                   IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> > +                   (val & 0xf));
> > +   c_can_object_put(dev, 0, msg_obj_no, IF_COMM_ALL);
> > +
> > +   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_info(dev->dev.parent,
> > +           "setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
> > +
> > +   ctrl_save = priv->read_reg(priv, &priv->regs->control);
> > +   priv->write_reg(priv, &priv->regs->control,
> > +                   ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > +   priv->write_reg(priv, &priv->regs->btr, reg_btr);
> > +   priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
> > +   priv->write_reg(priv, &priv->regs->control, ctrl_save);
> > +
> > +   return 0;
> > +}
> > +
> > +/*
> > + * Configure C_CAN message objects for Tx and Rx purposes:
> > + * C_CAN provides a total of 32 message objects that can be
> > +configured
> > + * either for Tx or Rx purposes. Here the first 16 message objects
> > +are used as
> > + * a reception FIFO. The end of reception FIFO is signified by the
> > +EoB bit
> > + * being SET. The remaining 16 message objects are kept aside for Tx
> purposes.
> > + * See user guide document for further details on configuring
> message
> > + * objects.
> > + */
> > +static void c_can_configure_msg_objects(struct net_device *dev) {
> > +   int i;
> > +
> > +   /* first invalidate all message objects */
> > +   for (i = 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); }
> > +
> > +/*
> > + * Configure C_CAN chip:
> > + * - enable/disable auto-retransmission
> > + * - set operating mode
> > + * - configure message objects
> > + */
> > +static void c_can_chip_config(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> > +           /* disable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_DISABLE_AR);
> > +   else
> > +           /* enable automatic retransmission */
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_ENABLE_AR);
> > +
> > +   if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> > +                                   CAN_CTRLMODE_LOOPBACK)) {
> > +           /* loopback + silent mode : useful for hot self-test */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test,
> > +                           TEST_LBACK | TEST_SILENT);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > +           /* loopback mode : useful for self-test function */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
> > +   } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> > +           /* silent mode : bus-monitoring mode */
> > +           priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +                           CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > +           priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
> > +   } else
> > +           /* normal mode*/
> > +           priv->write_reg(priv, &priv->regs->control,
> > +                           CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> > +
> > +   /* configure message objects */
> > +   c_can_configure_msg_objects(dev);
> > +}
> > +
> > +static void c_can_start(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* enable status change, error and module interrupts */
> > +   c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> > +
> > +   /* basic c_can configuration */
> > +   c_can_chip_config(dev);
> > +
> > +   priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > +   /* reset tx helper pointers */
> > +   priv->tx_next = priv->tx_echo = 0;
> > +}
> > +
> > +static void c_can_stop(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   /* set the state as STOPPED */
> > +   priv->can.state = CAN_STATE_STOPPED; }
> > +
> > +static int c_can_set_mode(struct net_device *dev, enum can_mode
> mode)
> > +{
> > +   switch (mode) {
> > +   case CAN_MODE_START:
> > +           c_can_start(dev);
> > +           netif_wake_queue(dev);
> > +           break;
> > +   default:
> > +           return -EOPNOTSUPP;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int c_can_get_berr_counter(const struct net_device *dev,
> > +                                   struct can_berr_counter *bec)
> > +{
> > +   unsigned int reg_err_counter;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +   bec->rxerr = (reg_err_counter & ERR_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->regs->txrqst1);
> > +           if (!(val & (1 << msg_obj_no))) {
> > +                   can_get_echo_skb(dev,
> > +                                   msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> > +                   stats->tx_bytes += priv->read_reg(priv,
> > +                                   &priv->regs->intf[0].msg_cntrl)
> > +                                   & IF_MCONT_DLC_MASK;
> > +                   stats->tx_packets++;
> > +           }
> > +   }
> > +
> > +   /* restart queue if wrap-up or if queue stalled on last pkt */
> > +   if (((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) != 0) ||
> > +                   ((priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) == 0))
> > +           netif_wake_queue(dev);
> > +}
> > +
> > +/*
> > + * theory of operation:
> > + *
> > + * c_can core saves a received CAN message into the first free
> > +message
> > + * object it finds free (starting with the lowest). Bits NEWDAT and
> > + * INTPND are set for this message object indicating that a new
> > +message
> > + * has arrived. To work-around this issue, we keep two groups of
> > +message
> > + * objects whose partitioning is defined by C_CAN_MSG_OBJ_RX_SPLIT.
> > + *
> > + * To ensure in-order frame reception we use the following
> > + * approach while re-activating a message object to receive further
> > + * frames:
> > + * - if the current message object number is lower than
> > + *   C_CAN_MSG_RX_LOW_LAST, do not clear the NEWDAT bit while
> clearing
> > + *   the INTPND bit.
> > + * - if the current message object number is equal to
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of all lower
> > + *   receive message objects.
> > + * - if the current message object number is greater than
> > + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of
> > + *   only this message object.
> > + */
> > +static int c_can_do_rx_poll(struct net_device *dev, int quota) {
> > +   u32 num_rx_pkts = 0;
> > +   unsigned int msg_obj, msg_ctrl_save;
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +   u32 val = c_can_read_reg32(priv, &priv->regs->intpnd1);
> > +
> > +   for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> > +                   msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
> > +                   msg_obj++) {
> > +           if (val & (1 << msg_obj)) {
>
> what about using find_next_bit here?

I will explore the possibility of using the same.
But, IMHO this logic seems much easier to understand than a
*for* loop with bulky *find_next_bit* call.

> > +                   c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
> > +                                   ~IF_COMM_TXRQST);
> > +                   msg_ctrl_save = priv->read_reg(priv,
> > +                                   &priv->regs->intf[0].msg_cntrl);
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_EOB)
> > +                           return num_rx_pkts;
> > +
> > +                   if (msg_ctrl_save & IF_MCONT_MSGLST) {
> > +                           c_can_handle_lost_msg_obj(dev, 0, msg_obj);
> > +                           num_rx_pkts++;
> > +                           quota--;
> > +                           continue;
> > +                   }
> > +
> > +                   if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
> > +                           continue;
> > +
> > +                   /* read the data from the message object */
> > +                   c_can_read_msg_object(dev, 0, msg_ctrl_save,
> msg_obj);
> > +
> > +                   if (msg_obj < C_CAN_MSG_RX_LOW_LAST)
> > +                           c_can_mark_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj > C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate this msg obj */
> > +                           c_can_activate_rx_msg_obj(dev, 0,
> > +                                           msg_ctrl_save, msg_obj);
> > +                   else if (msg_obj == C_CAN_MSG_RX_LOW_LAST)
> > +                           /* activate all lower message objects */
> > +                           c_can_activate_all_lower_rx_msg_obj(dev,
> > +                                           0, msg_ctrl_save);
> > +
> > +                   num_rx_pkts++;
> > +                   quota--;
> > +           }
> > +           val = c_can_read_reg32(priv, &priv->regs->intpnd1);
> > +   }
> > +
> > +   return num_rx_pkts;
> > +}
> > +
> > +static inline int c_can_has_and_handle_berr(struct c_can_priv *priv)
> > +{
> > +   return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> > +           (priv->current_status & STATUS_LEC_MASK); }
> > +
> > +static int c_can_err(struct net_device *dev,
> > +                           enum c_can_bus_error_types error_type,
> > +                           enum c_can_lec_type 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->regs->err_cnt);
> > +   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_enable_all_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
> > +    */
> > +
> > +   /* common for all type of bus errors */
> > +   priv->can.can_stats.bus_error++;
> > +   stats->rx_errors++;
> > +   cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > +   cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > +
> > +   switch (lec_type) {
> > +   case LEC_STUFF_ERROR:
> > +           dev_dbg(dev->dev.parent, "stuff error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_STUFF;
> > +           break;
> > +
> > +   case LEC_FORM_ERROR:
> > +           dev_dbg(dev->dev.parent, "form error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_FORM;
> > +           break;
> > +
> > +   case LEC_ACK_ERROR:
> > +           dev_dbg(dev->dev.parent, "ack error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> > +                           CAN_ERR_PROT_LOC_ACK_DEL);
> > +           break;
> > +
> > +   case LEC_BIT1_ERROR:
> > +           dev_dbg(dev->dev.parent, "bit1 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT1;
> > +           break;
> > +
> > +   case LEC_BIT0_ERROR:
> > +           dev_dbg(dev->dev.parent, "bit0 error\n");
> > +           cf->data[2] |= CAN_ERR_PROT_BIT0;
> > +           break;
> > +
> > +   case LEC_CRC_ERROR:
> > +           dev_dbg(dev->dev.parent, "CRC error\n");
> > +           cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > +                           CAN_ERR_PROT_LOC_CRC_DEL);
> > +           break;
> > +   }
> > +
> > +   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->regs->interrupt);
> > +
> > +   /* status events have the highest priority */
> > +   if (irqstatus == STATUS_INTERRUPT) {
> > +           priv->current_status = priv->read_reg(priv,
> > +                                   &priv->regs->status);
> > +
> > +           /* handle Tx/Rx events */
> > +           if (priv->current_status & STATUS_TXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_TXOK);
> > +
> > +           if (priv->current_status & STATUS_RXOK)
> > +                   priv->write_reg(priv, &priv->regs->status,
> > +                                   priv->current_status & ~STATUS_RXOK);
> > +
> > +           /* handle bus error events */
> > +           if (priv->current_status & STATUS_EWARN) {
> > +                   dev_dbg(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_dbg(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_dbg(dev->dev.parent,
> > +                                   "entered bus off state\n");
> > +                   error_type = C_CAN_BUS_OFF;
> > +           }
> > +
> > +           /* handle bus recovery events */
> > +           if ((!(priv->current_status & STATUS_EPASS)) &&
> > +                           (priv->last_status & STATUS_EPASS)) {
> > +                   dev_dbg(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_dbg(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 */
> > +           lec_type = c_can_has_and_handle_berr(priv);
> > +           if (lec_type && (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));
> > +   } 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_enable_all_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_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +   napi_schedule(&priv->napi);
> > +
> > +   return IRQ_HANDLED;
> > +}
>
> Have a look at the pch_can interrupt handler, it supports shared irqs.

Could you please send me a reference URL for the same.
As the pch_can driver code in David's net-next and net tree has almost
similar implementation to the c_can driver.
Or, am I missing something here?

> > +
> > +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,
> > +                           dev);
> > +   if (err < 0) {
> > +           dev_err(dev->dev.parent, "failed to request interrupt\n");
> > +           goto exit_irq_fail;
> > +   }
> > +
> > +   /* start the c_can controller */
> > +   c_can_start(dev);
> > +
> > +   napi_enable(&priv->napi);
> > +   netif_start_queue(dev);
> > +
> > +   return 0;
> > +
> > +exit_irq_fail:
> > +   close_candev(dev);
> > +   return err;
> > +}
> > +
> > +static int c_can_close(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   netif_stop_queue(dev);
> > +   napi_disable(&priv->napi);
> > +   c_can_stop(dev);
> > +   free_irq(dev->irq, dev);
> > +   close_candev(dev);
> > +
> > +   return 0;
> > +}
> > +
> > +struct net_device *alloc_c_can_dev(void)
>
> Please model after alloc_sja1000_dev:
>
> struct net_device *alloc_sja1000dev(int sizeof_priv)
>
> The private for the _user_ of alloc_c_can_dev is behind the sja1000
> private, so you can get rid of the void *priv member in the struct
> c_can_priv. (see below)

Ok.
But Wolfgang also suggested to use the *priv* member inside c_can_priv struct
for board specific details. In my c_can platform driver I use it to
store the *clk* variable. Do I need to change that as well?

> > +{
> > +   struct net_device *dev;
> > +   struct c_can_priv *priv;
> > +
> > +   dev = alloc_candev(sizeof(struct c_can_priv),
> C_CAN_MSG_OBJ_TX_NUM);
> > +   if (!dev)
> > +           return NULL;
> > +
> > +   priv = netdev_priv(dev);
> > +   netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> > +
> > +   priv->dev = dev;
> > +   priv->can.bittiming_const = &c_can_bittiming_const;
> > +   priv->can.do_set_bittiming = c_can_set_bittiming;
> > +   priv->can.do_set_mode = c_can_set_mode;
> > +   priv->can.do_get_berr_counter = c_can_get_berr_counter;
> > +   priv->can.ctrlmode_supported = CAN_CTRLMODE_ONE_SHOT |
> > +                                   CAN_CTRLMODE_LOOPBACK |
> > +                                   CAN_CTRLMODE_LISTENONLY |
> > +                                   CAN_CTRLMODE_BERR_REPORTING;
> > +
> > +   return dev;
> > +}
> > +EXPORT_SYMBOL_GPL(alloc_c_can_dev);
> > +
> > +void free_c_can_dev(struct net_device *dev) {
> > +   free_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(free_c_can_dev);
> > +
> > +static const struct net_device_ops c_can_netdev_ops = {
> > +   .ndo_open = c_can_open,
> > +   .ndo_stop = c_can_close,
> > +   .ndo_start_xmit = c_can_start_xmit,
> > +};
> > +
> > +int register_c_can_dev(struct net_device *dev) {
> > +   dev->flags |= IFF_ECHO; /* we support local echo */
> > +   dev->netdev_ops = &c_can_netdev_ops;
> > +
> > +   return register_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(register_c_can_dev);
> > +
> > +void unregister_c_can_dev(struct net_device *dev) {
> > +   struct c_can_priv *priv = netdev_priv(dev);
> > +
> > +   /* disable all interrupts */
> > +   c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> > +
> > +   unregister_candev(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(unregister_c_can_dev);
> > +
> > +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> > +MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("CAN bus driver for
> > +Bosch C_CAN controller");
> > diff --git a/drivers/net/can/c_can/c_can.h
> > b/drivers/net/can/c_can/c_can.h new file mode 100644 index
> > 0000000..6d24432
> > --- /dev/null
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -0,0 +1,235 @@
> > +/*
> > + * CAN bus driver for Bosch C_CAN controller
> > + *
> > + * Copyright (C) 2010 ST Microelectronics
> > + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> > + *
> > + * Borrowed heavily from the C_CAN driver originally written by:
> > + * Copyright (C) 2007
> > + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
> > +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> > + *
> > + * TX and RX NAPI implementation has been borrowed from at91 CAN
> > +driver
> > + * written by:
> > + * Copyright
> > + * (C) 2007 by Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> > + * (C) 2008, 2009 by Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> IMHO you can drop the at91 copyright reference here.

Ok.

> > + *
> > + * Bosch C_CAN controller is compliant to CAN protocol version 2.0
> part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + *
> http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef C_CAN_H
> > +#define C_CAN_H
> > +
> > +/* control register */
> > +#define CONTROL_TEST               BIT(7)
> > +#define CONTROL_CCE                BIT(6)
> > +#define CONTROL_DISABLE_AR BIT(5)
> > +#define CONTROL_ENABLE_AR  (0 << 5)
> > +#define CONTROL_EIE                BIT(3)
> > +#define CONTROL_SIE                BIT(2)
> > +#define CONTROL_IE         BIT(1)
> > +#define CONTROL_INIT               BIT(0)
> > +
> > +/* test register */
> > +#define TEST_RX                    BIT(7)
> > +#define TEST_TX1           BIT(6)
> > +#define TEST_TX2           BIT(5)
> > +#define TEST_LBACK         BIT(4)
> > +#define TEST_SILENT                BIT(3)
> > +#define TEST_BASIC         BIT(2)
> > +
> > +/* status register */
> > +#define STATUS_BOFF                BIT(7)
> > +#define STATUS_EWARN               BIT(6)
> > +#define STATUS_EPASS               BIT(5)
> > +#define STATUS_RXOK                BIT(4)
> > +#define STATUS_TXOK                BIT(3)
> > +#define STATUS_LEC_MASK            0x07
> > +
> > +/* error counter register */
> > +#define ERR_COUNTER_TEC_MASK       0xff
> > +#define ERR_COUNTER_TEC_SHIFT      0
> > +#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               BIT(15)
> > +
> > +/* IFx command mask */
> > +#define IF_COMM_WR         BIT(7)
> > +#define IF_COMM_MASK               BIT(6)
> > +#define IF_COMM_ARB                BIT(5)
> > +#define IF_COMM_CONTROL            BIT(4)
> > +#define IF_COMM_CLR_INT_PND        BIT(3)
> > +#define IF_COMM_TXRQST             BIT(2)
> > +#define IF_COMM_DATAA              BIT(1)
> > +#define IF_COMM_DATAB              BIT(0)
> > +#define IF_COMM_ALL                (IF_COMM_MASK | IF_COMM_ARB | \
> > +                           IF_COMM_CONTROL | IF_COMM_TXRQST | \
> > +                           IF_COMM_DATAA | IF_COMM_DATAB)
> > +
> > +/* IFx arbitration */
> > +#define IF_ARB_MSGVAL              BIT(15)
> > +#define IF_ARB_MSGXTD              BIT(14)
> > +#define IF_ARB_TRANSMIT            BIT(13)
> > +
> > +/* IFx message control */
> > +#define IF_MCONT_NEWDAT            BIT(15)
> > +#define IF_MCONT_MSGLST            BIT(14)
> > +#define IF_MCONT_CLR_MSGLST        (0 << 14)
> > +#define IF_MCONT_INTPND            BIT(13)
> > +#define IF_MCONT_UMASK             BIT(12)
> > +#define IF_MCONT_TXIE              BIT(11)
> > +#define IF_MCONT_RXIE              BIT(10)
> > +#define IF_MCONT_RMTEN             BIT(9)
> > +#define IF_MCONT_TXRQST            BIT(8)
> > +#define IF_MCONT_EOB               BIT(7)
> > +#define IF_MCONT_DLC_MASK  0xf
> > +
> > +/*
> > + * IFx register masks:
> > + * allow easy operation on 16-bit registers when the
> > + * argument is 32-bit instead
> > + */
> > +#define IFX_WRITE_LOW_16BIT(x)     ((x) & 0xFFFF)
> > +#define IFX_WRITE_HIGH_16BIT(x)    (((x) & 0xFFFF0000) >> 16)
> > +
> > +/* message object split */
> > +#define C_CAN_NO_OF_OBJECTS        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_MSG_OBJ_RX_SPLIT     8
> > +#define C_CAN_MSG_RX_LOW_LAST      (C_CAN_MSG_OBJ_RX_SPLIT - 1)
> > +
> > +#define C_CAN_NEXT_MSG_OBJ_MASK    (C_CAN_MSG_OBJ_TX_NUM - 1)
> > +#define RECEIVE_OBJECT_BITS        0x0000ffff
> > +
> > +/* status interrupt */
> > +#define STATUS_INTERRUPT   0x8000
> > +
> > +/* global interrupt masks */
> > +#define ENABLE_ALL_INTERRUPTS      1
> > +#define DISABLE_ALL_INTERRUPTS     0
> > +
> > +/* minimum timeout for checking BUSY status */
> > +#define MIN_TIMEOUT_VALUE  6
> > +
> > +/* napi related */
> > +#define C_CAN_NAPI_WEIGHT  C_CAN_MSG_OBJ_RX_NUM
> > +
> > +/* c_can IF registers */
> > +struct c_can_if_regs {
> > +   u16 com_reg;
> > +   u16 com_mask;
> > +   u16 mask1;
> > +   u16 mask2;
> > +   u16 arb1;
> > +   u16 arb2;
> > +   u16 msg_cntrl;
> > +   u16 data[4];
> > +   u16 _reserved[13];
> > +};
> > +
> > +/* c_can hardware registers */
> > +struct c_can_regs {
> > +   u16 control;
> > +   u16 status;
> > +   u16 err_cnt;
> > +   u16 btr;
> > +   u16 interrupt;
> > +   u16 test;
> > +   u16 brp_ext;
> > +   u16 _reserved1;
> > +   struct c_can_if_regs intf[2]; /* [0] = IF1 and [1] = IF2 */
> > +   u16 _reserved2[8];
> > +   u16 txrqst1;
> > +   u16 txrqst2;
> > +   u16 _reserved3[6];
> > +   u16 newdat1;
> > +   u16 newdat2;
> > +   u16 _reserved4[6];
> > +   u16 intpnd1;
> > +   u16 intpnd2;
> > +   u16 _reserved5[6];
> > +   u16 msgval1;
> > +   u16 msgval2;
> > +   u16 _reserved6[6];
> > +};
> > +
> > +/* c_can lec values */
> > +enum c_can_lec_type {
> > +   LEC_STUFF_ERROR = 1,
> > +   LEC_FORM_ERROR,
> > +   LEC_ACK_ERROR,
> > +   LEC_BIT1_ERROR,
> > +   LEC_BIT0_ERROR,
> > +   LEC_CRC_ERROR,
> > +};
> > +
> > +/*
> > + * 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,
> > +};
> > +
> > +/* c_can private data structure */
> > +struct c_can_priv {
> > +   struct can_priv can;    /* must be the first member */
> > +   struct napi_struct napi;
> > +   struct net_device *dev;
> > +   int tx_object;
> > +   int current_status;
> > +   int last_status;
> > +   u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> > +   void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> > +   struct c_can_regs __iomem *regs;
> > +   unsigned long irq_flags; /* for request_irq() */
> > +   unsigned int tx_next;
> > +   unsigned int tx_echo;
> > +   void *priv;             /* for board-specific data */
>         ^^^^^^^^^^
>
> here

Please see above.

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

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Marc Kleine-Budde @ 2011-01-12  8:53 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
	Wolfram Sang, Oliver Hartkopp
In-Reply-To: <4D2D6ABE.7000405-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>


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

On 01/12/2011 09:47 AM, Wolfgang Grandegger wrote:
> On 01/11/2011 11:01 PM, Marc Kleine-Budde wrote:
>> On 01/11/2011 07:25 PM, Wolfgang Grandegger wrote:
>>
>> [...]
>>
>>> I was told that Bosch's page for the C_CAN is here:
>>>
>>>  http://www.semiconductors.bosch.de/en/ipmodules/can/canipmodules/c_can/c_can.asp
>>>
>>> There it's also written for what bus interfaces the C_CAN is available for, e.g.:
>>>
>>>   "The ASIC version is delivered with the AMBA APB bus interface from ARM."
>>
>> On ARM we also have the AMBA bus abstraction in Linux, but I've never
>> worked with it. IIRC Wolfram (Cc'ed) has recently touched it.
>>
>>> which is obviously used in the "Platform Controller Hub" eg20t.
>>
>> But in the pch, they've attached the AMBA to the PCI bus.
> 
> Ah, right. Anyway, with that driver we are prepared to handle other bus
> interfaces by adding another bus-sensitive driver in
> "drivers/net/can/c_can".

ACK.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2011-01-12  9:08 UTC (permalink / raw)
  To: Bhupesh SHARMA
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Kleine-Budde,
	David Miller
In-Reply-To: <D5ECB3C7A6F99444980976A8C6D896384DEAFA31B6-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>

On 01/12/2011 09:51 AM, Bhupesh SHARMA wrote:
> Hi Marc,
> 
> Thanks for your review.
> Please see my comments inline:
> 
>> -----Original Message-----
>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
...
>>> +static int c_can_close(struct net_device *dev) {
>>> +   struct c_can_priv *priv = netdev_priv(dev);
>>> +
>>> +   netif_stop_queue(dev);
>>> +   napi_disable(&priv->napi);
>>> +   c_can_stop(dev);
>>> +   free_irq(dev->irq, dev);
>>> +   close_candev(dev);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +struct net_device *alloc_c_can_dev(void)
>>
>> Please model after alloc_sja1000_dev:
>>
>> struct net_device *alloc_sja1000dev(int sizeof_priv)
>>
>> The private for the _user_ of alloc_c_can_dev is behind the sja1000
>> private, so you can get rid of the void *priv member in the struct
>> c_can_priv. (see below)
> 
> Ok.
> But Wolfgang also suggested to use the *priv* member inside c_can_priv struct
> for board specific details. In my c_can platform driver I use it to
> store the *clk* variable. Do I need to change that as well?

Marc is referring to:

http://lxr.linux.no/#linux+v2.6.37/drivers/net/can/sja1000/sja1000.c#L582

But also there "priv->priv" is used to store a pointer to the board
specific details. Looking to the SJA1000 drivers, only *one* uses
"sizeof_priv > 0", but many other attach a separately allocated
structure to "priv->priv". For that reason, I'm fine with your current
implementation.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Marc Kleine-Budde @ 2011-01-12  9:24 UTC (permalink / raw)
  To: Bhupesh SHARMA
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
	Wolfgang Grandegger
In-Reply-To: <D5ECB3C7A6F99444980976A8C6D896384DEAFA31B6-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>


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

On 01/12/2011 09:51 AM, Bhupesh SHARMA wrote:

[..]

>>> b/drivers/net/can/c_can/c_can.c new file mode 100644 index
>>> 0000000..06e1553
>>> --- /dev/null
>>> +++ b/drivers/net/can/c_can/c_can.c
>>> @@ -0,0 +1,953 @@
>>> +/*
>>> + * CAN bus driver for Bosch C_CAN controller
>>> + *
>>> + * Copyright (C) 2010 ST Microelectronics
>>> + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
>>> + *
>>> + * Borrowed heavily from the C_CAN driver originally written by:
>>> + * Copyright (C) 2007
>>> + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
>>> +<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
>>> + *
>>> + * TX and RX NAPI implementation has been borrowed from at91 CAN
>>> +driver
>>> + * written by:
>>
>> I've just cleaned up the RX implementation, have a look at [1] and [2].
> 
> I am not sure that C_CAN will be effected as per the at91 driver changes.
> In C_CAN, messages numbers are allowed only from 0x01 to 0x20. Message object
> number 0 is not allowed. Internally obj-put and obj-get routines increment the
> message number by 1.

Okay - I just wanted you to have a look at it.

[...]

>>> +/*
>>> + * theory of operation:
>>> + *
>>> + * c_can core saves a received CAN message into the first free
>>> +message
>>> + * object it finds free (starting with the lowest). Bits NEWDAT and
>>> + * INTPND are set for this message object indicating that a new
>>> +message
>>> + * has arrived. To work-around this issue, we keep two groups of
>>> +message
>>> + * objects whose partitioning is defined by C_CAN_MSG_OBJ_RX_SPLIT.
>>> + *
>>> + * To ensure in-order frame reception we use the following
>>> + * approach while re-activating a message object to receive further
>>> + * frames:
>>> + * - if the current message object number is lower than
>>> + *   C_CAN_MSG_RX_LOW_LAST, do not clear the NEWDAT bit while
>> clearing
>>> + *   the INTPND bit.
>>> + * - if the current message object number is equal to
>>> + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of all lower
>>> + *   receive message objects.
>>> + * - if the current message object number is greater than
>>> + *   C_CAN_MSG_RX_LOW_LAST then clear the NEWDAT bit of
>>> + *   only this message object.
>>> + */
>>> +static int c_can_do_rx_poll(struct net_device *dev, int quota) {
>>> +   u32 num_rx_pkts = 0;
>>> +   unsigned int msg_obj, msg_ctrl_save;
>>> +   struct c_can_priv *priv = netdev_priv(dev);
>>> +   u32 val = c_can_read_reg32(priv, &priv->regs->intpnd1);
>>> +
>>> +   for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
>>> +                   msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
>>> +                   msg_obj++) {
>>> +           if (val & (1 << msg_obj)) {
>>
>> what about using find_next_bit here?
> 
> I will explore the possibility of using the same.
> But, IMHO this logic seems much easier to understand than a
> *for* loop with bulky *find_next_bit* call.

:)

>>> +                   c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
>>> +                                   ~IF_COMM_TXRQST);
>>> +                   msg_ctrl_save = priv->read_reg(priv,
>>> +                                   &priv->regs->intf[0].msg_cntrl);
>>> +
>>> +                   if (msg_ctrl_save & IF_MCONT_EOB)
>>> +                           return num_rx_pkts;
>>> +
>>> +                   if (msg_ctrl_save & IF_MCONT_MSGLST) {
>>> +                           c_can_handle_lost_msg_obj(dev, 0, msg_obj);
>>> +                           num_rx_pkts++;
>>> +                           quota--;
>>> +                           continue;
>>> +                   }
>>> +
>>> +                   if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
>>> +                           continue;
>>> +
>>> +                   /* read the data from the message object */
>>> +                   c_can_read_msg_object(dev, 0, msg_ctrl_save,
>> msg_obj);
>>> +
>>> +                   if (msg_obj < C_CAN_MSG_RX_LOW_LAST)
>>> +                           c_can_mark_rx_msg_obj(dev, 0,
>>> +                                           msg_ctrl_save, msg_obj);
>>> +                   else if (msg_obj > C_CAN_MSG_RX_LOW_LAST)
>>> +                           /* activate this msg obj */
>>> +                           c_can_activate_rx_msg_obj(dev, 0,
>>> +                                           msg_ctrl_save, msg_obj);
>>> +                   else if (msg_obj == C_CAN_MSG_RX_LOW_LAST)
>>> +                           /* activate all lower message objects */
>>> +                           c_can_activate_all_lower_rx_msg_obj(dev,
>>> +                                           0, msg_ctrl_save);
>>> +
>>> +                   num_rx_pkts++;
>>> +                   quota--;
>>> +           }
>>> +           val = c_can_read_reg32(priv, &priv->regs->intpnd1);
>>> +   }
>>> +
>>> +   return num_rx_pkts;
>>> +}
>>> +
>>> +static inline int c_can_has_and_handle_berr(struct c_can_priv *priv)
>>> +{
>>> +   return (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
>>> +           (priv->current_status & STATUS_LEC_MASK); }
>>> +
>>> +static int c_can_err(struct net_device *dev,
>>> +                           enum c_can_bus_error_types error_type,
>>> +                           enum c_can_lec_type 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->regs->err_cnt);
>>> +   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_enable_all_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
>>> +    */
>>> +
>>> +   /* common for all type of bus errors */
>>> +   priv->can.can_stats.bus_error++;
>>> +   stats->rx_errors++;
>>> +   cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +   cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>>> +
>>> +   switch (lec_type) {
>>> +   case LEC_STUFF_ERROR:
>>> +           dev_dbg(dev->dev.parent, "stuff error\n");
>>> +           cf->data[2] |= CAN_ERR_PROT_STUFF;
>>> +           break;
>>> +
>>> +   case LEC_FORM_ERROR:
>>> +           dev_dbg(dev->dev.parent, "form error\n");
>>> +           cf->data[2] |= CAN_ERR_PROT_FORM;
>>> +           break;
>>> +
>>> +   case LEC_ACK_ERROR:
>>> +           dev_dbg(dev->dev.parent, "ack error\n");
>>> +           cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
>>> +                           CAN_ERR_PROT_LOC_ACK_DEL);
>>> +           break;
>>> +
>>> +   case LEC_BIT1_ERROR:
>>> +           dev_dbg(dev->dev.parent, "bit1 error\n");
>>> +           cf->data[2] |= CAN_ERR_PROT_BIT1;
>>> +           break;
>>> +
>>> +   case LEC_BIT0_ERROR:
>>> +           dev_dbg(dev->dev.parent, "bit0 error\n");
>>> +           cf->data[2] |= CAN_ERR_PROT_BIT0;
>>> +           break;
>>> +
>>> +   case LEC_CRC_ERROR:
>>> +           dev_dbg(dev->dev.parent, "CRC error\n");
>>> +           cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
>>> +                           CAN_ERR_PROT_LOC_CRC_DEL);
>>> +           break;
>>> +   }
>>> +
>>> +   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->regs->interrupt);
>>> +
>>> +   /* status events have the highest priority */
>>> +   if (irqstatus == STATUS_INTERRUPT) {
>>> +           priv->current_status = priv->read_reg(priv,
>>> +                                   &priv->regs->status);
>>> +
>>> +           /* handle Tx/Rx events */
>>> +           if (priv->current_status & STATUS_TXOK)
>>> +                   priv->write_reg(priv, &priv->regs->status,
>>> +                                   priv->current_status & ~STATUS_TXOK);
>>> +
>>> +           if (priv->current_status & STATUS_RXOK)
>>> +                   priv->write_reg(priv, &priv->regs->status,
>>> +                                   priv->current_status & ~STATUS_RXOK);
>>> +
>>> +           /* handle bus error events */
>>> +           if (priv->current_status & STATUS_EWARN) {
>>> +                   dev_dbg(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_dbg(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_dbg(dev->dev.parent,
>>> +                                   "entered bus off state\n");
>>> +                   error_type = C_CAN_BUS_OFF;
>>> +           }
>>> +
>>> +           /* handle bus recovery events */
>>> +           if ((!(priv->current_status & STATUS_EPASS)) &&
>>> +                           (priv->last_status & STATUS_EPASS)) {
>>> +                   dev_dbg(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_dbg(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 */
>>> +           lec_type = c_can_has_and_handle_berr(priv);
>>> +           if (lec_type && (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));
>>> +   } 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_enable_all_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_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
>>> +   napi_schedule(&priv->napi);
>>> +
>>> +   return IRQ_HANDLED;
>>> +}
>>
>> Have a look at the pch_can interrupt handler, it supports shared irqs.
> 
> Could you please send me a reference URL for the same.
> As the pch_can driver code in David's net-next and net tree has almost
> similar implementation to the c_can driver.
> Or, am I missing something here?

Your interrupt handler unconditionally thinks it's a c_can interrupt...

http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=blob;f=drivers/net/can/pch_can.c;h=c42e97268248889acdb007a16cb9bf8152413bd2;hb=0c21e3aaf6ae85bee804a325aa29c325209180fd#l581

...but the pch_can checks if it's really a interrupt from the c_can core.

If you implement your interrupt handler properly, you can register the
interrupt handler as SHARED:

http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=blob;f=drivers/net/can/pch_can.c;h=c42e97268248889acdb007a16cb9bf8152413bd2;hb=0c21e3aaf6ae85bee804a325aa29c325209180fd#l850

regards, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH net-next-2.6 v4 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Marc Kleine-Budde @ 2011-01-12  9:30 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Bhupesh SHARMA, Socketcan-core@lists.berlios.de,
	netdev@vger.kernel.org, David Miller
In-Reply-To: <4D2D6F95.8030502@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

On 01/12/2011 10:08 AM, Wolfgang Grandegger wrote:
> On 01/12/2011 09:51 AM, Bhupesh SHARMA wrote:
>> Hi Marc,
>>
>> Thanks for your review.
>> Please see my comments inline:
>>
>>> -----Original Message-----
>>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
>>> On 01/11/2011 12:49 PM, Bhupesh Sharma wrote:
> ...
>>>> +static int c_can_close(struct net_device *dev) {
>>>> +   struct c_can_priv *priv = netdev_priv(dev);
>>>> +
>>>> +   netif_stop_queue(dev);
>>>> +   napi_disable(&priv->napi);
>>>> +   c_can_stop(dev);
>>>> +   free_irq(dev->irq, dev);
>>>> +   close_candev(dev);
>>>> +
>>>> +   return 0;
>>>> +}
>>>> +
>>>> +struct net_device *alloc_c_can_dev(void)
>>>
>>> Please model after alloc_sja1000_dev:
>>>
>>> struct net_device *alloc_sja1000dev(int sizeof_priv)
>>>
>>> The private for the _user_ of alloc_c_can_dev is behind the sja1000
>>> private, so you can get rid of the void *priv member in the struct
>>> c_can_priv. (see below)
>>
>> Ok.
>> But Wolfgang also suggested to use the *priv* member inside c_can_priv struct
>> for board specific details. In my c_can platform driver I use it to
>> store the *clk* variable. Do I need to change that as well?
> 
> Marc is referring to:
> 
> http://lxr.linux.no/#linux+v2.6.37/drivers/net/can/sja1000/sja1000.c#L582
> 
> But also there "priv->priv" is used to store a pointer to the board
> specific details. Looking to the SJA1000 drivers, only *one* uses
> "sizeof_priv > 0", but many other attach a separately allocated
> structure to "priv->priv". For that reason, I'm fine with your current
> implementation.

Okay fine with me.

A nice cleanup might be to introduce something like this:

static inline void * give_me_my_priv_from_sja1000_priv
	(struct sja1000_priv *priv)
{
	return (void *)(priv + 1);
}

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply


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