* Re: [PATCH v2 2/2] netlink: support setting devgroup parameters
From: jamal @ 2011-01-12 15:44 UTC (permalink / raw)
To: Vlad Dogaru; +Cc: netdev, Octavian Purdila
In-Reply-To: <1294763724-9927-3-git-send-email-ddvlad@rosedu.org>
On Tue, 2011-01-11 at 18:35 +0200, Vlad Dogaru wrote:
> If a rtnetlink request specifies a negative or zero ifindex and has no
> interface name attribute, but has a group attribute, then the chenges
> are made to all the interfaces belonging to the specified group.
>
> Signed-off-by: Vlad Dogaru <ddvlad@rosedu.org>
Looks good. Please just do some basic tests like setting a few
interfaces to the same group, changing their MTU and setting admin
up then down. If it works add my ACKed-by on both patches.
The only thing that will still generate a lot of noise is the netlink
events that will be generated afterwards for each change on a netdev in
a group i.e a single netlink message..
Maybe we could batch those in a future patch
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2 1/2] net_device: add support for network device groups
From: jamal @ 2011-01-12 15:40 UTC (permalink / raw)
To: Vlad Dogaru; +Cc: netdev, Octavian Purdila
In-Reply-To: <1294763724-9927-2-git-send-email-ddvlad@rosedu.org>
On Tue, 2011-01-11 at 18:35 +0200, Vlad Dogaru wrote:
> Net devices can now be grouped, enabling simpler manipulation from
> userspace. This patch adds a group field to the net_device strucure, as
^^^^^
typo
> well as rtnetlink support to query and modify it.
>
> + unsigned int group;
Should that be int?
cheers,
jamal
^ permalink raw reply
* [patch] bridge-utils: show selected bridge
From: Anton Danilov @ 2011-01-12 15:29 UTC (permalink / raw)
To: netdev
diff --git a/brctl/brctl_cmd.c b/brctl/brctl_cmd.c
index d37e99c..d95fe54 100644
--- a/brctl/brctl_cmd.c
+++ b/brctl/brctl_cmd.c
@@ -338,8 +338,14 @@ static int show_bridge(const char *name, void *arg)
static int br_cmd_show(int argc, char *const* argv)
{
+ int i = 0;
+
printf("bridge name\tbridge id\t\tSTP enabled\tinterfaces\n");
- br_foreach_bridge(show_bridge, NULL);
+ if (argc == 1)
+ br_foreach_bridge(show_bridge, NULL);
+ else
+ for(i = 2; i <= argc; i++)
+ show_bridge(argv[i - 1], NULL);
return 0;
}
@@ -454,7 +460,8 @@ static const struct command commands[] = {
"<bridge> <port> <cost>\tset path cost" },
{ 3, "setportprio", br_cmd_setportprio,
"<bridge> <port> <prio>\tset port priority" },
- { 0, "show", br_cmd_show, "\t\t\tshow a list of bridges" },
+ { 0, "show", br_cmd_show,
+ "[ <bridge> ]\t\tshow a list of bridges" },
{ 1, "showmacs", br_cmd_showmacs,
"<bridge>\t\tshow a list of mac addrs"},
{ 1, "showstp", br_cmd_showstp,
--
Anton.
^ permalink raw reply related
* [PATCH] net: remove dev_txq_stats_fold()
From: Eric Dumazet @ 2011-01-12 15:02 UTC (permalink / raw)
To: Jarek Poplawski, David Miller
Cc: David Brownell, Michał Nazarewicz, Neil Jones, linux-usb,
netdev, Alexander Duyck, Jeff Kirsher
In-Reply-To: <1294840379.3981.31.camel@edumazet-laptop>
Le mercredi 12 janvier 2011 à 14:52 +0100, Eric Dumazet a écrit :
> Or even better, remove these counters since there is no users left but
> ixgbe.
>
> (vlans, tunnels, ... now use percpu stats)
>
>
Thanks Jarek for the reminder :)
[PATCH] net: remove dev_txq_stats_fold()
After recent changes, (percpu stats on vlan/tunnels...), we dont need
anymore per struct netdev_queue tx_bytes/tx_packets/tx_dropped counters.
Only remaining users are ixgbe, sch_teql & macvlan :
1) ixgbe can be converted to use existing tx_ring counters.
2) macvlan incremented txq->tx_dropped, it can use the
dev->stats.tx_dropped counter.
3) sch_teql : almost revert ab35cd4b8f42 (Use net_device internal stats)
Now we have ndo_get_stats64(), use it.
This removes a lockdep warning (and possible lockup) in rndis gadget,
calling dev_get_stats() from hard IRQ context.
Ref: http://www.spinics.net/lists/netdev/msg149202.html
Reported-by: Neil Jones <neiljay@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jarek Poplawski <jarkao2@gmail.com>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 23 ++++++++++++++++-------
drivers/net/macvtap.c | 2 +-
include/linux/netdevice.h | 5 -----
net/core/dev.c | 29 -----------------------------
net/sched/sch_teql.c | 26 +++++++++++++++++++++-----
5 files changed, 38 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index a060610..602078b 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6667,8 +6667,6 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
struct ixgbe_adapter *adapter,
struct ixgbe_ring *tx_ring)
{
- struct net_device *netdev = tx_ring->netdev;
- struct netdev_queue *txq;
unsigned int first;
unsigned int tx_flags = 0;
u8 hdr_len = 0;
@@ -6765,9 +6763,6 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
/* add the ATR filter if ATR is on */
if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))
ixgbe_atr(tx_ring, skb, tx_flags, protocol);
- txq = netdev_get_tx_queue(netdev, tx_ring->queue_index);
- txq->tx_bytes += skb->len;
- txq->tx_packets++;
ixgbe_tx_queue(tx_ring, tx_flags, count, skb->len, hdr_len);
ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
@@ -6925,8 +6920,6 @@ static struct rtnl_link_stats64 *ixgbe_get_stats64(struct net_device *netdev,
struct ixgbe_adapter *adapter = netdev_priv(netdev);
int i;
- /* accurate rx/tx bytes/packets stats */
- dev_txq_stats_fold(netdev, stats);
rcu_read_lock();
for (i = 0; i < adapter->num_rx_queues; i++) {
struct ixgbe_ring *ring = ACCESS_ONCE(adapter->rx_ring[i]);
@@ -6943,6 +6936,22 @@ static struct rtnl_link_stats64 *ixgbe_get_stats64(struct net_device *netdev,
stats->rx_bytes += bytes;
}
}
+
+ for (i = 0; i < adapter->num_tx_queues; i++) {
+ struct ixgbe_ring *ring = ACCESS_ONCE(adapter->tx_ring[i]);
+ u64 bytes, packets;
+ unsigned int start;
+
+ if (ring) {
+ do {
+ start = u64_stats_fetch_begin_bh(&ring->syncp);
+ packets = ring->stats.packets;
+ bytes = ring->stats.bytes;
+ } while (u64_stats_fetch_retry_bh(&ring->syncp, start));
+ stats->tx_packets += packets;
+ stats->tx_bytes += bytes;
+ }
+ }
rcu_read_unlock();
/* following stats updated by ixgbe_watchdog_task() */
stats->multicast = netdev->stats.multicast;
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 21845af..5933621 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -585,7 +585,7 @@ err:
rcu_read_lock_bh();
vlan = rcu_dereference(q->vlan);
if (vlan)
- netdev_get_tx_queue(vlan->dev, 0)->tx_dropped++;
+ vlan->dev->stats.tx_dropped++;
rcu_read_unlock_bh();
return err;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index be4957c..d971346 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -520,9 +520,6 @@ struct netdev_queue {
* please use this field instead of dev->trans_start
*/
unsigned long trans_start;
- u64 tx_bytes;
- u64 tx_packets;
- u64 tx_dropped;
} ____cacheline_aligned_in_smp;
static inline int netdev_queue_numa_node_read(const struct netdev_queue *q)
@@ -2265,8 +2262,6 @@ extern void dev_load(struct net *net, const char *name);
extern void dev_mcast_init(void);
extern struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
struct rtnl_link_stats64 *storage);
-extern void dev_txq_stats_fold(const struct net_device *dev,
- struct rtnl_link_stats64 *stats);
extern int netdev_max_backlog;
extern int netdev_tstamp_prequeue;
diff --git a/net/core/dev.c b/net/core/dev.c
index a3ef808..83507c2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5523,34 +5523,6 @@ void netdev_run_todo(void)
}
}
-/**
- * dev_txq_stats_fold - fold tx_queues stats
- * @dev: device to get statistics from
- * @stats: struct rtnl_link_stats64 to hold results
- */
-void dev_txq_stats_fold(const struct net_device *dev,
- struct rtnl_link_stats64 *stats)
-{
- u64 tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
- unsigned int i;
- struct netdev_queue *txq;
-
- for (i = 0; i < dev->num_tx_queues; i++) {
- txq = netdev_get_tx_queue(dev, i);
- spin_lock_bh(&txq->_xmit_lock);
- tx_bytes += txq->tx_bytes;
- tx_packets += txq->tx_packets;
- tx_dropped += txq->tx_dropped;
- spin_unlock_bh(&txq->_xmit_lock);
- }
- if (tx_bytes || tx_packets || tx_dropped) {
- stats->tx_bytes = tx_bytes;
- stats->tx_packets = tx_packets;
- stats->tx_dropped = tx_dropped;
- }
-}
-EXPORT_SYMBOL(dev_txq_stats_fold);
-
/* Convert net_device_stats to rtnl_link_stats64. They have the same
* fields in the same order, with only the type differing.
*/
@@ -5594,7 +5566,6 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
netdev_stats_to_stats64(storage, ops->ndo_get_stats(dev));
} else {
netdev_stats_to_stats64(storage, &dev->stats);
- dev_txq_stats_fold(dev, storage);
}
storage->rx_dropped += atomic_long_read(&dev->rx_dropped);
return storage;
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index af9360d..8b82c13 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -59,6 +59,10 @@ struct teql_master
struct net_device *dev;
struct Qdisc *slaves;
struct list_head master_list;
+ u64 tx_bytes;
+ u64 tx_packets;
+ u64 tx_errors;
+ u64 tx_dropped;
};
struct teql_sched_data
@@ -274,7 +278,6 @@ static inline int teql_resolve(struct sk_buff *skb,
static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct teql_master *master = netdev_priv(dev);
- struct netdev_queue *txq = netdev_get_tx_queue(dev, 0);
struct Qdisc *start, *q;
int busy;
int nores;
@@ -314,8 +317,8 @@ restart:
__netif_tx_unlock(slave_txq);
master->slaves = NEXT_SLAVE(q);
netif_wake_queue(dev);
- txq->tx_packets++;
- txq->tx_bytes += length;
+ master->tx_packets++;
+ master->tx_bytes += length;
return NETDEV_TX_OK;
}
__netif_tx_unlock(slave_txq);
@@ -342,10 +345,10 @@ restart:
netif_stop_queue(dev);
return NETDEV_TX_BUSY;
}
- dev->stats.tx_errors++;
+ master->tx_errors++;
drop:
- txq->tx_dropped++;
+ master->tx_dropped++;
dev_kfree_skb(skb);
return NETDEV_TX_OK;
}
@@ -398,6 +401,18 @@ static int teql_master_close(struct net_device *dev)
return 0;
}
+static struct rtnl_link_stats64 *teql_master_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *stats)
+{
+ struct teql_master *m = netdev_priv(dev);
+
+ stats->tx_packets = m->tx_packets;
+ stats->tx_bytes = m->tx_bytes;
+ stats->tx_errors = m->tx_errors;
+ stats->tx_dropped = m->tx_dropped;
+ return stats;
+}
+
static int teql_master_mtu(struct net_device *dev, int new_mtu)
{
struct teql_master *m = netdev_priv(dev);
@@ -422,6 +437,7 @@ static const struct net_device_ops teql_netdev_ops = {
.ndo_open = teql_master_open,
.ndo_stop = teql_master_close,
.ndo_start_xmit = teql_master_xmit,
+ .ndo_get_stats64 = teql_master_stats64,
.ndo_change_mtu = teql_master_mtu,
};
^ permalink raw reply related
* Re: [PATCH v4 08/10] ARM: mxs: add ocotp read function
From: Sascha Hauer @ 2011-01-12 14:50 UTC (permalink / raw)
To: Shawn Guo
Cc: davem, gerg, baruch, eric, bryan.wu, r64343, B32542,
u.kleine-koenig, lw, w.sang, jamie, jamie, netdev,
linux-arm-kernel
In-Reply-To: <20110112064711.GG2888@freescale.com>
On Wed, Jan 12, 2011 at 02:47:12PM +0800, Shawn Guo wrote:
> 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);
>
> ......
> }
I would rather make a function like this:
static u32 ocotp[0x27];
const u32 *mxs_get_ocotp(void)
{
static int once = 0;
if (once)
return ocotp
/* bank open */
for (i = 0; i < 0x27; i++)
ocotp[i] = readl(ocotp_base + 0x20 + i * 0x10)
/* bank_close */
once = 1;
return ocotp;
}
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: panic in tg3 driver
From: Stephen Clark @ 2011-01-12 13:53 UTC (permalink / raw)
To: Matt Carlson; +Cc: Linux Kernel Network Developers, Michael Chan
In-Reply-To: <20110112030652.GA27164@mcarlson.broadcom.com>
On 01/11/2011 10:06 PM, Matt Carlson wrote:
> lspci -vvv -xxx -s 81:00.0
Linux Z1010.netwolves.com 2.6.37 #9 SMP PREEMPT Wed Jan 5 11:14:46 EST
2011 i686
i686 i386 GNU/Linux
[root@Z1010 ~]# lspci -vvv -xxx -s 81:00.0
81:00.0 Ethernet controller: Broadcom Corporation NetLink BCM5906M Fast
Ethernet
PCI Express (rev 02)
Subsystem: Broadcom Corporation Unknown device 9713
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepp
ing- SERR- FastB2B-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <
MAbort- >SERR- <PERR-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 10
Region 0: Memory at cfbf0000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [48] Power Management version 3
Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot+
,D3cold+)
Status: D0 PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] Vital Product Data
Capabilities: [58] Vendor Specific Information
Capabilities: [e8] Message Signalled Interrupts: 64bit+
Queue=0/0 Enable-
Address: 00000000fee0100c Data: 4169
Capabilities: [d0] Express Endpoint IRQ 0
Device: Supported: MaxPayload 128 bytes, PhantFunc 0,
ExtTag+
Device: Latency L0s <4us, L1 unlimited
Device: AtnBtn- AtnInd- PwrInd-
Device: Errors: Correctable- Non-Fatal- Fatal- Unsupported-
Device: RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
Device: MaxPayload 128 bytes, MaxReadReq 512 bytes
Link: Supported Speed 2.5Gb/s, Width x1, ASPM L0s L1,
Port 0
Link: Latency L0s <4us, L1 <64us
Link: ASPM Disabled RCB 64 bytes CommClk- ExtSynch-
Link: Speed 2.5Gb/s, Width x1
Capabilities: [100] Advanced Error Reporting
Capabilities: [13c] Virtual Channel
Capabilities: [160] Device Serial Number 39-d1-36-fe-ff-b6-02-00
00: e4 14 13 17 06 00 10 00 02 00 00 02 10 00 00 00
10: 04 00 bf cf 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 e4 14 13 97
30: 00 00 00 00 48 00 00 00 00 00 00 00 0a 01 00 00
40: 00 00 00 00 00 00 00 00 01 50 03 c0 08 00 00 00
50: 03 58 fc 00 78 00 00 00 09 e8 78 00 64 2f 72 64
60: 00 00 00 00 00 00 00 00 00 00 02 c0 00 00 00 00
70: 12 12 00 00 a0 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 fe 50 08 24
90: 01 92 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 10 00 01 00 a0 8f 64 00 00 20 10 00 11 6c 03 00
e0: 00 00 11 10 00 00 00 00 05 d0 80 00 0c 10 e0 fe
f0: 00 00 00 00 69 41 00 00 00 00 00 00 00 00 00 00
modprobe tg3
dmesg output:
tg3.c:v3.115 (October 14, 2010)
tg3 0000:81:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
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 addr
ess 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 -> GSI 16 (level, low) -> IRQ 16
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 addr
ess 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]
[root@Z1010 ~]# ethtool -i eth2
driver: tg3
version: 3.115
firmware-version: sb v3.03
bus-info: 0000:81:00.0
[root@Z1010 ~]# cat /proc/interrupts
CPU0
0: 173 IO-APIC-edge timer
1: 2 IO-APIC-edge i8042
4: 2864 IO-APIC-edge serial
6: 2 IO-APIC-edge floppy
8: 0 IO-APIC-edge rtc0
9: 0 IO-APIC-fasteoi acpi
14: 0 IO-APIC-edge pata_via
15: 8100 IO-APIC-edge pata_via
16: 984 IO-APIC-fasteoi eth0
17: 104 IO-APIC-fasteoi eth1
20: 0 IO-APIC-fasteoi uhci_hcd:usb2
21: 0 IO-APIC-fasteoi uhci_hcd:usb4, sata_via
22: 0 IO-APIC-fasteoi ehci_hcd:usb1, uhci_hcd:usb3
23: 0 IO-APIC-fasteoi uhci_hcd:usb5
NMI: 0 Non-maskable interrupts
LOC: 101963 Local timer interrupts
SPU: 0 Spurious interrupts
PMI: 0 Performance monitoring interrupts
IWI: 0 IRQ work interrupts
RES: 0 Rescheduling interrupts
CAL: 0 Function call interrupts
TLB: 0 TLB shootdowns
TRM: 0 Thermal event interrupts
THR: 0 Threshold APIC interrupts
MCE: 0 Machine check exceptions
MCP: 0 Machine check polls
ERR: 0
MIS: 0
The b44 interfaces are working great.
[root@Z1010 ~]# ifconfig eth2 up
do_IRQ: 0.64 No irq handler for vector (irq -1)
system becomes unresponsive then ususally
reboots.
but it didn't this last time just has become really doggy in
responding
[root@Z1010 ~]#
[root@Z1010 ~]#
[root@Z1010 ~]# ifconfig
eth0 Link encap:Ethernet HWaddr 00:02:B6:36:D1:37
inet addr:10.0.129.4 Bcast:10.0.255.255 Mask:255.255.128.0
inet6 addr: fe80::202:b6ff:fe36:d137/64 Scope:Link
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:1025 errors:0 dropped:12 overruns:0 frame:0
TX packets:6 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:185675 (181.3 KiB) TX bytes:492 (492.0 b)
Interrupt:16
eth1 Link encap:Ethernet HWaddr 00:02:B6:36:D1:38
inet6 addr: fe80::202:b6ff:fe36:d138/64 Scope:Link
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:35 errors:0 dropped:0 overruns:0 frame:0
TX packets:41 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:2612 (2.5 KiB) TX bytes:4014 (3.9 KiB)
Interrupt:17
eth2 Link encap:Ethernet HWaddr 00:02:B6:36:D1:39
UP BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 b) TX bytes:0 (0.0 b)
Interrupt:16
lo Link encap:Local Loopback
inet addr:127.0.0.1 Mask:255.0.0.0
inet6 addr: ::1/128 Scope:Host
UP LOOPBACK RUNNING MTU:16436 Metric:1
RX packets:5298 errors:0 dropped:0 overruns:0 frame:0
TX packets:5298 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:0
RX bytes:475525 (464.3 KiB) TX bytes:475525 (464.3 KiB)
Message from syslogd@ at Wed Jan 12 08:44:17 2011 ...
localhost kernel: do_IRQ: 0.192 No irq handler for vector (irq -1)
Message from syslogd@ at Wed Jan 12 08:44:17 2011 ...
localhost kernel: do_IRQ: 0.64 No irq handler for vector (irq -1)
[root@Z1010 ~]# cat /proc/interrupts
CPU0
0: 173 IO-APIC-edge timer
1: 2 IO-APIC-edge i8042
4: 821 IO-APIC-edge serial
6: 2 IO-APIC-edge floppy
8: 0 IO-APIC-edge rtc0
9: 2 IO-APIC-fasteoi acpi
14: 0 IO-APIC-edge pata_via
15: 19522 IO-APIC-edge pata_via
16: 256 IO-APIC-fasteoi eth0, eth2
17: 54 IO-APIC-fasteoi eth1
20: 0 IO-APIC-fasteoi uhci_hcd:usb2
21: 0 IO-APIC-fasteoi uhci_hcd:usb4, sata_via
22: 0 IO-APIC-fasteoi ehci_hcd:usb1, uhci_hcd:usb3
23: 0 IO-APIC-fasteoi uhci_hcd:usb5
NMI: 0 Non-maskable interrupts
LOC: 116090 Local timer interrupts
SPU: 0 Spurious interrupts
PMI: 0 Performance monitoring interrupts
IWI: 0 IRQ work interrupts
RES: 0 Rescheduling interrupts
CAL: 0 Function call interrupts
TLB: 0 TLB shootdowns
TRM: 0 Thermal event interrupts
THR: 0 Threshold APIC interrupts
MCE: 0 Machine check exceptions
MCP: 0 Machine check polls
ERR: 38
MIS: 2
[root@Z1010 ~]# arp -an
the system has now lost ethernet connectivity via the b44 ports
This is a test system and I can recompile the kernel if there are
any patches you would like me to try out.
^ permalink raw reply
* Re: rndis gadget: Inconsistent locking
From: Eric Dumazet @ 2011-01-12 13:52 UTC (permalink / raw)
To: Jarek Poplawski
Cc: David Brownell, Michał Nazarewicz, Neil Jones,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1294839140.3981.23.camel@edumazet-laptop>
Le mercredi 12 janvier 2011 à 14:32 +0100, Eric Dumazet a écrit :
> Le mercredi 12 janvier 2011 à 13:23 +0000, Jarek Poplawski a écrit :
> > On Wed, Jan 12, 2011 at 02:07:12PM +0100, Eric Dumazet wrote:
> > ...
> > >
> > > Hmm...
> > >
> > > So all net devices in gen_ndis_query_resp() should have a
> > > ndo_get_stats() or ndo_get_stats64() method, not allowed to use
> > > spin_lock_bh() / spin_unlock_bh()
> > >
> > > If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
> > > tries to revert your patch ;)
> >
> > I'm not sure I got your point: my patch could be replaced with
> > ndo_get_stats64() implementing irq safe locking or by changing
> > gen_ndis_query_resp() calling context. It's intended as a fast
> > (compatible) fix.
>
> I was mentioning that we tried in past months to remove useless
> ndo_get_stats() methods that were only doing :
>
> return &net->stats;
>
> random commit : b27d50a9ff5cf2775b7a4daf5
>
> Another possibility would be to use u64_stats_sync.h for these txq
> counters.
>
> (no locking needed to read counters, only a seqcount fetch/retry)
>
> As a bonus, no overhead on 64bit arches.
>
Or even better, remove these counters since there is no users left but
ixgbe.
(vlans, tunnels, ... now use percpu stats)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: rndis gadget: Inconsistent locking
From: Jarek Poplawski @ 2011-01-12 13:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Brownell, Michał Nazarewicz, Neil Jones,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1294839140.3981.23.camel@edumazet-laptop>
On Wed, Jan 12, 2011 at 02:32:20PM +0100, Eric Dumazet wrote:
> Le mercredi 12 janvier 2011 ?? 13:23 +0000, Jarek Poplawski a écrit :
> > On Wed, Jan 12, 2011 at 02:07:12PM +0100, Eric Dumazet wrote:
> > ...
> > >
> > > Hmm...
> > >
> > > So all net devices in gen_ndis_query_resp() should have a
> > > ndo_get_stats() or ndo_get_stats64() method, not allowed to use
> > > spin_lock_bh() / spin_unlock_bh()
> > >
> > > If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
> > > tries to revert your patch ;)
> >
> > I'm not sure I got your point: my patch could be replaced with
> > ndo_get_stats64() implementing irq safe locking or by changing
> > gen_ndis_query_resp() calling context. It's intended as a fast
> > (compatible) fix.
>
> I was mentioning that we tried in past months to remove useless
> ndo_get_stats() methods that were only doing :
>
> return &net->stats;
>
> random commit : b27d50a9ff5cf2775b7a4daf5
>
> Another possibility would be to use u64_stats_sync.h for these txq
> counters.
>
> (no locking needed to read counters, only a seqcount fetch/retry)
>
> As a bonus, no overhead on 64bit arches.
Well, I'm OK with anything, but this thread gets older and older ;-)
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: rndis gadget: Inconsistent locking
From: Eric Dumazet @ 2011-01-12 13:32 UTC (permalink / raw)
To: Jarek Poplawski
Cc: David Brownell, Michał Nazarewicz, Neil Jones, linux-usb,
netdev
In-Reply-To: <20110112132314.GA9920@ff.dom.local>
Le mercredi 12 janvier 2011 à 13:23 +0000, Jarek Poplawski a écrit :
> On Wed, Jan 12, 2011 at 02:07:12PM +0100, Eric Dumazet wrote:
> ...
> >
> > Hmm...
> >
> > So all net devices in gen_ndis_query_resp() should have a
> > ndo_get_stats() or ndo_get_stats64() method, not allowed to use
> > spin_lock_bh() / spin_unlock_bh()
> >
> > If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
> > tries to revert your patch ;)
>
> I'm not sure I got your point: my patch could be replaced with
> ndo_get_stats64() implementing irq safe locking or by changing
> gen_ndis_query_resp() calling context. It's intended as a fast
> (compatible) fix.
I was mentioning that we tried in past months to remove useless
ndo_get_stats() methods that were only doing :
return &net->stats;
random commit : b27d50a9ff5cf2775b7a4daf5
Another possibility would be to use u64_stats_sync.h for these txq
counters.
(no locking needed to read counters, only a seqcount fetch/retry)
As a bonus, no overhead on 64bit arches.
^ permalink raw reply
* Re: rndis gadget: Inconsistent locking
From: Jarek Poplawski @ 2011-01-12 13:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Brownell, Michał Nazarewicz, Neil Jones, linux-usb,
netdev
In-Reply-To: <1294837632.3981.18.camel@edumazet-laptop>
On Wed, Jan 12, 2011 at 02:07:12PM +0100, Eric Dumazet wrote:
...
>
> Hmm...
>
> So all net devices in gen_ndis_query_resp() should have a
> ndo_get_stats() or ndo_get_stats64() method, not allowed to use
> spin_lock_bh() / spin_unlock_bh()
>
> If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
> tries to revert your patch ;)
I'm not sure I got your point: my patch could be replaced with
ndo_get_stats64() implementing irq safe locking or by changing
gen_ndis_query_resp() calling context. It's intended as a fast
(compatible) fix.
Jarek P.
^ permalink raw reply
* Re: rndis gadget: Inconsistent locking
From: Eric Dumazet @ 2011-01-12 13:07 UTC (permalink / raw)
To: Jarek Poplawski
Cc: David Brownell, Michał Nazarewicz, Neil Jones,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110112122811.GA9513-8HppEYmqbBCE+EvaaNYduQ@public.gmane.org>
Le mercredi 12 janvier 2011 à 12:28 +0000, Jarek Poplawski a écrit :
> On 2011-01-10 13:14, David Brownell wrote:
> >
> >
> >> I have just retested Michals patch but I have found another
> >> lockdep failure.
> >>
> >> It looks like rndis_msg_parser() can call dev_get_stats> too:
> >
> > Can someone provide a fully working patch then?
> > Or maybe just revert the one that caused all the
> > breakage??
> >
> > Rememeber that this driver was working fine for
> > years before netdev changes added multiple bugs
> > because (at least) they didn't update all callers.
>
> Maybe I miss something, but if it's like Michal described something
> like this should be enough (not tested nor compiled).
>
> Jarek P.
> ---
>
> drivers/usb/gadget/f_phonet.c | 6 ++++++
> drivers/usb/gadget/u_ether.c | 6 ++++++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
> index 3c6e1a0..bef4622 100644
> --- a/drivers/usb/gadget/f_phonet.c
> +++ b/drivers/usb/gadget/f_phonet.c
> @@ -207,6 +207,11 @@ static int pn_net_close(struct net_device *dev)
> return 0;
> }
>
> +static struct net_device_stats *pn_net_stats(struct net_device *dev)
> +{
> + return &dev->stats;
> +}
> +
> static void pn_tx_complete(struct usb_ep *ep, struct usb_request *req)
> {
> struct f_phonet *fp = ep->driver_data;
> @@ -279,6 +284,7 @@ static int pn_net_mtu(struct net_device *dev, int new_mtu)
> static const struct net_device_ops pn_netdev_ops = {
> .ndo_open = pn_net_open,
> .ndo_stop = pn_net_close,
> + .ndo_get_stats = pn_net_stats,
> .ndo_start_xmit = pn_net_xmit,
> .ndo_change_mtu = pn_net_mtu,
> };
> diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
> index 1eda968..f21520f 100644
> --- a/drivers/usb/gadget/u_ether.c
> +++ b/drivers/usb/gadget/u_ether.c
> @@ -702,6 +702,11 @@ static int eth_stop(struct net_device *net)
> return 0;
> }
>
> +static struct net_device_stats *eth_get_stats(struct net_device *net)
> +{
> + return &net->stats;
> +}
> +
> /*-------------------------------------------------------------------------*/
>
> /* initial value, changed by "ifconfig usb0 hw ether xx:xx:xx:xx:xx:xx" */
> @@ -740,6 +745,7 @@ static struct eth_dev *the_dev;
> static const struct net_device_ops eth_netdev_ops = {
> .ndo_open = eth_open,
> .ndo_stop = eth_stop,
> + .ndo_get_stats = eth_get_stats,
> .ndo_start_xmit = eth_start_xmit,
> .ndo_change_mtu = ueth_change_mtu,
> .ndo_set_mac_address = eth_mac_addr,
> --
Hmm...
So all net devices in gen_ndis_query_resp() should have a
ndo_get_stats() or ndo_get_stats64() method, not allowed to use
spin_lock_bh() / spin_unlock_bh()
If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
tries to revert your patch ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: rndis gadget: Inconsistent locking
From: Jarek Poplawski @ 2011-01-12 12:28 UTC (permalink / raw)
To: David Brownell
Cc: =?ISO-8859-2?Q?Micha=B3_Nazarewicz?=, Neil Jones, linux-usb,
netdev
In-Reply-To: <466657.40223.qm@web180306.mail.gq1.yahoo.com>
On 2011-01-10 13:14, David Brownell wrote:
>
>
>> I have just retested Michals patch but I have found another
>> lockdep failure.
>>
>> It looks like rndis_msg_parser() can call dev_get_stats> too:
>
> Can someone provide a fully working patch then?
> Or maybe just revert the one that caused all the
> breakage??
>
> Rememeber that this driver was working fine for
> years before netdev changes added multiple bugs
> because (at least) they didn't update all callers.
Maybe I miss something, but if it's like Michal described something
like this should be enough (not tested nor compiled).
Jarek P.
---
drivers/usb/gadget/f_phonet.c | 6 ++++++
drivers/usb/gadget/u_ether.c | 6 ++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index 3c6e1a0..bef4622 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -207,6 +207,11 @@ static int pn_net_close(struct net_device *dev)
return 0;
}
+static struct net_device_stats *pn_net_stats(struct net_device *dev)
+{
+ return &dev->stats;
+}
+
static void pn_tx_complete(struct usb_ep *ep, struct usb_request *req)
{
struct f_phonet *fp = ep->driver_data;
@@ -279,6 +284,7 @@ static int pn_net_mtu(struct net_device *dev, int new_mtu)
static const struct net_device_ops pn_netdev_ops = {
.ndo_open = pn_net_open,
.ndo_stop = pn_net_close,
+ .ndo_get_stats = pn_net_stats,
.ndo_start_xmit = pn_net_xmit,
.ndo_change_mtu = pn_net_mtu,
};
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 1eda968..f21520f 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -702,6 +702,11 @@ static int eth_stop(struct net_device *net)
return 0;
}
+static struct net_device_stats *eth_get_stats(struct net_device *net)
+{
+ return &net->stats;
+}
+
/*-------------------------------------------------------------------------*/
/* initial value, changed by "ifconfig usb0 hw ether xx:xx:xx:xx:xx:xx" */
@@ -740,6 +745,7 @@ static struct eth_dev *the_dev;
static const struct net_device_ops eth_netdev_ops = {
.ndo_open = eth_open,
.ndo_stop = eth_stop,
+ .ndo_get_stats = eth_get_stats,
.ndo_start_xmit = eth_start_xmit,
.ndo_change_mtu = ueth_change_mtu,
.ndo_set_mac_address = eth_mac_addr,
^ permalink raw reply related
* Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro
From: Richard Cochran @ 2011-01-12 12:17 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, netdev, Alan Cox, Arnd Bergmann,
Christoph Lameter, David Miller, John Stultz, Krzysztof Halasa,
Peter Zijlstra, Rodolfo Giometti
In-Reply-To: <alpine.LFD.2.00.1101121201080.12146@localhost6.localdomain6>
On Wed, Jan 12, 2011 at 12:16:10PM +0100, Thomas Gleixner wrote:
>
> We probably can remove the dispatch inlines that way completely and do
> the call directly from the syscall function.
Right, okay. I also wanted to put the whole logic into the syscall
itself. I'll try it that way.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro
From: Thomas Gleixner @ 2011-01-12 11:16 UTC (permalink / raw)
To: Richard Cochran
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: <20110112073728.GA2935-7KxsofuKt4IfAd9E5cN8NEzG7cXyKsk/@public.gmane.org>
On Wed, 12 Jan 2011, Richard Cochran wrote:
> 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.
Right, It's about lines of code and ever repeated if / else constructs
in the dispatch functions. The number of comparisons is probably the
same, but I'm sure that the binary size will be smaller.
We probably can remove the dispatch inlines that way completely and do
the call directly from the syscall function.
> > 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.
Well, if we document clock_get_array_id() proper, then it's easier to
follow than 10 dispatch functions which have all the same checks and
comparisons inside.
> 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!
These IDs are not public, they are helpers to make the code simpler,
nothing else. I agree, that we don't want to extend the static ids for
public consumption, but implementation wise we can do that confined to
posix-timers.c.
> Having said that, if you insist on it, I won't mind reworking the
> dispatch code as you suggested.
I'm not insisting. I just saw the repeated if/else constructs and
added the clockfd check mentally :) That's where I started to wonder
about a way to do the same thing with way less lines of code.
> > > 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
Missed that, sorry.
> argument! I have removed the macro for the next round.
Cool.
Thanks,
tglx
^ permalink raw reply
* [PATCH] ipv4: devconf: start IPV4_DEVCONF_* from 0
From: Lucian Adrian Grijincu @ 2011-01-12 10:19 UTC (permalink / raw)
To: netdev, Thomas Graf
Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
Octavian Purdila, Vlad Dogaru
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
The IPV4_DEVCONF_* enums are never exposed to the userspace and it
would make code simpler to remove all the useless (-1) adjustments.
-----
Thomas Graf added an interface that exposes the enums (counting from 1)
for libnl which seems to be the only user of the interface:
commit 9f0f7272ac9506f4c8c05cc597b7e376b0b9f3e4
Author: Thomas Graf <tgraf@infradead.org>
Date: Tue Nov 16 04:32:48 2010 +0000
ipv4: AF_INET link address family
The development branch of libnl[0] redefines the enums in userspace
counting from 1 (to mimic the 2.6.37 kernel).
The stable[1] version of libnl does not define or use these enums.
This patch adjusts the number received from userspace to not break the
ABI, but given these circumstances, could we consider the 2.6.37 ABI
unstable and change the only user to start counting from 0?
[0] http://git.kernel.org/?p=libs/netlink/libnl.git
[1] http://git.kernel.org/?p=libs/netlink/libnl-stable.git
-----
And the moral of the story is that we had better regard — after
all those centuries! — zero as a most natural number.
http://www.cs.utexas.edu/users/EWD/transcriptions/EWD08xx/EWD831.html
CC: Edsger Wybe Dijkstra <dijkstra@cs.utexas.edu>
CC: Thomas Graf <tgraf@infradead.org>
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
include/linux/inetdevice.h | 9 +++------
net/ipv4/devinet.c | 44 ++++++++++++++++++++++++++------------------
2 files changed, 29 insertions(+), 24 deletions(-)
[-- Attachment #2: 0003-ipv4-devconf-start-IPV4_DEVCONF_-from-0.patch --]
[-- Type: text/x-patch, Size: 5017 bytes --]
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ae8fdc5..e4d93b2 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -13,7 +13,7 @@
enum
{
- IPV4_DEVCONF_FORWARDING=1,
+ IPV4_DEVCONF_FORWARDING,
IPV4_DEVCONF_MC_FORWARDING,
IPV4_DEVCONF_PROXY_ARP,
IPV4_DEVCONF_ACCEPT_REDIRECTS,
@@ -38,10 +38,9 @@ enum
IPV4_DEVCONF_ACCEPT_LOCAL,
IPV4_DEVCONF_SRC_VMARK,
IPV4_DEVCONF_PROXY_ARP_PVLAN,
- __IPV4_DEVCONF_MAX
+ IPV4_DEVCONF_MAX
};
-#define IPV4_DEVCONF_MAX (__IPV4_DEVCONF_MAX - 1)
struct ipv4_devconf {
void *sysctl;
@@ -72,20 +71,18 @@ struct in_device {
struct rcu_head rcu_head;
};
-#define IPV4_DEVCONF(cnf, attr) ((cnf).data[IPV4_DEVCONF_ ## attr - 1])
+#define IPV4_DEVCONF(cnf, attr) ((cnf).data[IPV4_DEVCONF_ ## attr])
#define IPV4_DEVCONF_ALL(net, attr) \
IPV4_DEVCONF((*(net)->ipv4.devconf_all), attr)
static inline int ipv4_devconf_get(struct in_device *in_dev, int index)
{
- index--;
return in_dev->cnf.data[index];
}
static inline void ipv4_devconf_set(struct in_device *in_dev, int index,
int val)
{
- index--;
set_bit(index, in_dev->cnf.state);
in_dev->cnf.data[index] = val;
}
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 748cb5b..181d558 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -65,20 +65,20 @@
static struct ipv4_devconf ipv4_devconf = {
.data = {
- [IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
+ [IPV4_DEVCONF_ACCEPT_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SEND_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SECURE_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SHARED_MEDIA] = 1,
},
};
static struct ipv4_devconf ipv4_devconf_dflt = {
.data = {
- [IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
- [IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
- [IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
+ [IPV4_DEVCONF_ACCEPT_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SEND_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SECURE_REDIRECTS] = 1,
+ [IPV4_DEVCONF_SHARED_MEDIA] = 1,
+ [IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE] = 1,
},
};
@@ -1304,12 +1304,15 @@ static int inet_validate_link_af(const struct net_device *dev,
if (tb[IFLA_INET_CONF]) {
nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
- int cfgid = nla_type(a);
+ /* Userspace indexes these ids starting from 1.
+ * This was the way the kernel indexed elements too,
+ * but now it counts from 0 */
+ int cfgid = nla_type(a) - 1;
if (nla_len(a) < 4)
return -EINVAL;
- if (cfgid <= 0 || cfgid > IPV4_DEVCONF_MAX)
+ if (cfgid < 0 || cfgid >= IPV4_DEVCONF_MAX)
return -EINVAL;
}
}
@@ -1330,8 +1333,13 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla)
BUG();
if (tb[IFLA_INET_CONF]) {
- nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
- ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
+ nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
+ /* Userspace indexes these ids starting from 1.
+ * This was the way the kernel indexed elements too,
+ * but now it counts from 0 */
+ int cfgid = nla_type(a) - 1;
+ ipv4_devconf_set(in_dev, cfgid, nla_get_u32(a));
+ }
}
return 0;
@@ -1449,7 +1457,7 @@ static int ipv4_doint_and_flush(ctl_table *ctl, int write,
{ \
.procname = name, \
.data = ipv4_devconf.data + \
- IPV4_DEVCONF_ ## attr - 1, \
+ IPV4_DEVCONF_ ## attr, \
.maxlen = sizeof(int), \
.mode = mval, \
.proc_handler = proc, \
@@ -1470,7 +1478,7 @@ static int ipv4_doint_and_flush(ctl_table *ctl, int write,
static struct devinet_sysctl_table {
struct ctl_table_header *sysctl_header;
- struct ctl_table devinet_vars[__IPV4_DEVCONF_MAX];
+ struct ctl_table devinet_vars[IPV4_DEVCONF_MAX + 1];
char *dev_name;
} devinet_sysctl = {
.devinet_vars = {
@@ -1505,6 +1513,7 @@ static struct devinet_sysctl_table {
"force_igmp_version"),
DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
"promote_secondaries"),
+ { }
},
};
@@ -1590,8 +1599,7 @@ static void devinet_sysctl_unregister(struct in_device *idev)
static struct ctl_table ctl_forward_entry[] = {
{
.procname = "ip_forward",
- .data = &ipv4_devconf.data[
- IPV4_DEVCONF_FORWARDING - 1],
+ .data = &ipv4_devconf.data[IPV4_DEVCONF_FORWARDING],
.maxlen = sizeof(int),
.mode = 0644,
.proc_handler = devinet_sysctl_forward,
@@ -1635,7 +1643,7 @@ static __net_init int devinet_init_net(struct net *net)
if (tbl == NULL)
goto err_alloc_ctl;
- tbl[0].data = &all->data[IPV4_DEVCONF_FORWARDING - 1];
+ tbl[0].data = &all->data[IPV4_DEVCONF_FORWARDING];
tbl[0].extra1 = all;
tbl[0].extra2 = net;
#endif
^ permalink raw reply related
* [PATCH net-next-2.6] netdev: tilepro: Use is_multicast_ether_addr helper
From: Tobias Klauser @ 2011-01-12 9:31 UTC (permalink / raw)
To: Chris Metcalf, netdev
Use is_multicast_ether_addr from linux/etherdevice.h instead of a custom
macro. Also remove the broadcast address check, as it is considered a
multicast address too.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/tile/tilepro.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tile/tilepro.c b/drivers/net/tile/tilepro.c
index 0e6bac5..8d8b67b 100644
--- a/drivers/net/tile/tilepro.c
+++ b/drivers/net/tile/tilepro.c
@@ -142,14 +142,6 @@
MODULE_AUTHOR("Tilera");
MODULE_LICENSE("GPL");
-
-#define IS_MULTICAST(mac_addr) \
- (((u8 *)(mac_addr))[0] & 0x01)
-
-#define IS_BROADCAST(mac_addr) \
- (((u16 *)(mac_addr))[0] == 0xffff)
-
-
/*
* Queue of incoming packets for a specific cpu and device.
*
@@ -795,7 +787,7 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/*
* FIXME: Implement HW multicast filter.
*/
- if (!IS_MULTICAST(buf) && !IS_BROADCAST(buf)) {
+ if (!is_multicast_ether_addr(buf)) {
/* Filter packets not for our address. */
const u8 *mine = dev->dev_addr;
filter = compare_ether_addr(mine, buf);
--
1.7.0.4
^ permalink raw reply related
* [PATCH net-next-2.6] netdev: ucc_geth: Use is_multicast_ether_addr helper
From: Tobias Klauser @ 2011-01-12 9:31 UTC (permalink / raw)
To: Li Yang, netdev; +Cc: linuxppc-dev
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/ucc_geth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 73a3e0d..715e7b4 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -2032,7 +2032,7 @@ static void ucc_geth_set_multi(struct net_device *dev)
netdev_for_each_mc_addr(ha, dev) {
/* Only support group multicast for now.
*/
- if (!(ha->addr[0] & 1))
+ if (!is_multicast_ether_addr(ha->addr))
continue;
/* Ask CPM to run CRC and set bit in
--
1.7.0.4
^ permalink raw reply related
* [PATCH net-next-2.6] netdev: bfin_mac: Use is_multicast_ether_addr helper
From: Tobias Klauser @ 2011-01-12 9:30 UTC (permalink / raw)
To: Michael Hennerich, uclinux-dist-devel, netdev
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/bfin_mac.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 0b9fc51..fe75e7a 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -1293,7 +1293,7 @@ static void bfin_mac_multicast_hash(struct net_device *dev)
addrs = ha->addr;
/* skip non-multicast addresses */
- if (!(*addrs & 1))
+ if (!is_multicast_ether_addr(addrs))
continue;
crc = ether_crc(ETH_ALEN, addrs);
--
1.7.0.4
^ permalink raw reply related
* 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
* 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: 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 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: 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: 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: 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox