* Re: Problem with non aligned DMA in usbnet on ARM
From: Martin Fuzzey @ 2010-08-11 10:11 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: linux-usb, netdev, linux-arm-kernel
In-Reply-To: <20100811095454.GA22543@n2100.arm.linux.org.uk>
> You haven't said what HCD you're using.
>
imx21_hcd (submitted by me and mainlined in 2.6.34)
> In that case, I'd say the HCD must handle misaligned URBs itself so
> behaviour is consistent between all implementations on different
> platforms.
>
Here is a pointer to the thread where it was stated that HCD's don't
have to handle this.
http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
^ permalink raw reply
* Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35
From: Daniel Baluta @ 2010-08-11 10:37 UTC (permalink / raw)
To: Masayuki Ohtak
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
gregkh-l3A5Bk7waGM, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
meego-dev-WXzIur8shnEAvxtiuMwx3w, arjan-VuQAYsv1563Yd54FQh9/CA,
Wolfgang Grandegger
In-Reply-To: <4C61EDE5.4030505-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Hi,
2010/8/11 Masayuki Ohtak <masa-korg-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>:
> CAN driver of Topcliff PCH
>
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus.
> Topcliff PCH has CAN I/F. This driver enables CAN function.
>
> Signed-off-by: Masayuki Ohtake <masa-korg-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
I have a few questions:
1. Is your code based on Intel's CAN EP80579 ([1]) ?
2. Why don't you use kernel existing kfifo infrastructure? ([2]).
3. Can you provide a pointer to Topcliff PCH CAN I/F datasheet ?
thanks,
Daniel.
[1] http://www.intel.com/design/intarch/ep80579/sw_tools.htm
[2] http://lxr.linux.no/#linux+v2.6.35/kernel/kfifo.c
^ permalink raw reply
* Re: [PATCH] netpoll: use non-BH variant of RCU
From: Herbert Xu @ 2010-08-11 11:03 UTC (permalink / raw)
To: David Miller; +Cc: paulmck, linville, netdev
In-Reply-To: <20100810.163117.241919476.davem@davemloft.net>
On Tue, Aug 10, 2010 at 04:31:17PM -0700, David Miller wrote:
>
> Linus's tree is fine for this.
>
> But we need this fix for 2.6.35-stable too. Herbert would
> you object to putting John's more simple fix there?
To use rcu_read_lock safely we'd also need to add do synchronize_rcu
in addition of synchronize_rcu_bh, right Paul?
Of course as we were doing this unsafely prior to my patch anyway
I'm also fine with just reverting it.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] netpoll: use non-BH variant of RCU
From: Herbert Xu @ 2010-08-11 11:27 UTC (permalink / raw)
To: David Miller; +Cc: paulmck, linville, netdev
In-Reply-To: <20100811110330.GA6440@gondor.apana.org.au>
On Wed, Aug 11, 2010 at 07:03:30AM -0400, Herbert Xu wrote:
>
> To use rcu_read_lock safely we'd also need to add do synchronize_rcu
> in addition of synchronize_rcu_bh, right Paul?
>
> Of course as we were doing this unsafely prior to my patch anyway
> I'm also fine with just reverting it.
Actually, we could just disable IRQs for stable. How about this
patch (untested)?
netpoll: Disable IRQ around RCU dereference in netpoll_rx
We cannot use rcu_dereference_bh safely in netpoll_rx as we may
be called with IRQs disabled. We could however simply disable
IRQs as that too causes BH to be disabled and is safe in either
case.
Thanks to John Linville for discovering this bug and providing
a patch.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 413742c..54f286b 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,20 +63,20 @@ static inline bool netpoll_rx(struct sk_buff *skb)
unsigned long flags;
bool ret = false;
- rcu_read_lock_bh();
+ local_irq_save(flags);
npinfo = rcu_dereference_bh(skb->dev->npinfo);
if (!npinfo || (list_empty(&npinfo->rx_np) && !npinfo->rx_flags))
goto out;
- spin_lock_irqsave(&npinfo->rx_lock, flags);
+ spin_lock(&npinfo->rx_lock);
/* check rx_flags again with the lock held */
if (npinfo->rx_flags && __netpoll_rx(skb))
ret = true;
- spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+ spin_unlock(&npinfo->rx_lock);
out:
- rcu_read_unlock_bh();
+ local_irq_restore(flags);
return ret;
}
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: Problem with non aligned DMA in usbnet on ARM
From: Martin Fuzzey @ 2010-08-11 11:38 UTC (permalink / raw)
To: Matthieu CASTET
Cc: linux-usb, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <4C627479.4060400-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
On Wed, Aug 11, 2010 at 11:59 AM, Matthieu CASTET
<matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org> wrote:
> It is best to solve in usb driver, because for example in Asix case the
> driver already do copy (see asix_rx_fixup/asix_tx_fixup).
>
Yes, however those functions are only called for devices which register them.
Looking at the driver_info and product id tables shows that it's only
a subset of the devices that asix supports (88772 and 88178 chips)
In fact my hardware _is_ included
// DLink DUB-E100 H/W Ver B1 Alternate
USB_DEVICE (0x2001, 0x3c05),
.driver_info = (unsigned long) &ax88772_info,
but fixing this in asix_rx_fixup wouldn't solve it for the other
supported devices.
It seems to me these fixup functions are not intended to solve
alignment issues but rather implement device specific framing (such as
when the hardware packs multiple ethernet frames into a single urb)
> PS : what hcd driver do you use ?
imx21_hcd
Martin
--
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
* [PATCH] net: Fix a memmove bug in dev_gro_receive()
From: Jarek Poplawski @ 2010-08-11 12:02 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org, herbert@gondor.apana.org.au,
davem@davemloft.net
In-Reply-To: <20100810083426.GA11509@ff.dom.local>
[was: Re: Is it a possible bug in dev_gro_receive()?]
On Tue, Aug 10, 2010 at 08:34:26AM +0000, Jarek Poplawski wrote:
> On Tue, Aug 10, 2010 at 04:11:54PM +0800, Xin, Xiaohui wrote:
> > Jarek,
> > Seems community agree with your patch more.
> > So may you send out your patch then? Thanks!
> > Some of my related patches still need this fix.
>
> Hmm... But there was no my patch. Only a tiny, cosmetical suggestion
> to your patch. I'd be glad if you add some credit or my "Acked-by",
> of course. But if you really have a big problem, e.g. you don't like
> my suggestion, please confirm.
Hmm#2... OK, it's probably something with my English, but since it
seems to take too long, here it is. Xiaohui, I hope you'll send your
"Signed-off-by" at least.
Thanks,
Jarek P.
PS: I know, there is a bit too long line...
--------------------------->
>Xin Xiaohui wrote:
> I looked into the code dev_gro_receive(), found the code here:
> if the frags[0] is pulled to 0, then the page will be released,
> and memmove() frags left.
> Is that right? I'm not sure if memmove do right or not, but
> frags[0].size is never set after memove at least. what I think
> a simple way is not to do anything if we found frags[0].size == 0.
> The patch is as followed.
...
This version of the patch fixes the bug directly in memmove.
Reported-by: "Xin, Xiaohui" <xiaohui.xin@intel.com>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ae6543..3721fbb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3143,7 +3143,7 @@ pull:
put_page(skb_shinfo(skb)->frags[0].page);
memmove(skb_shinfo(skb)->frags,
skb_shinfo(skb)->frags + 1,
- --skb_shinfo(skb)->nr_frags);
+ --skb_shinfo(skb)->nr_frags * sizeof(skb_frag_t));
}
}
^ permalink raw reply related
* [PATCH NEXT 3/8] qlcnic: fix for setting function modes
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman, Rajesh Borundia
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
From: Rajesh Borundia <rajesh.borundia@qlogic.com>
function modes was not working with CNA device, in CNA mode
other function(FCOE) can be enabled before nic.
Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_main.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index bac2371..49c8110 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -533,8 +533,6 @@ qlcnic_set_function_modes(struct qlcnic_adapter *adapter)
ret = qlcnic_api_lock(adapter);
if (ret)
goto err_lock;
- if (QLC_DEV_CLR_REF_CNT(ref_count, adapter->ahw.pci_func))
- goto err_npar;
if (qlcnic_config_npars) {
for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
@@ -552,7 +550,6 @@ qlcnic_set_function_modes(struct qlcnic_adapter *adapter)
adapter->ahw.pci_func));
}
writel(data, priv_op);
-err_npar:
qlcnic_api_unlock(adapter);
err_lock:
return ret;
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 0/8]qlcnic: enhancement and fixes
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman
Hi
Series of 8 patches to fix minor bugs and to add statistics
support. Please apply them on net-next branch.
-Amit
^ permalink raw reply
* [PATCH NEXT 4/8] qlcnic: add eswitch statistics support
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
Adding eswitch statistics support. User can get
whole eswitch stats or stats of func belong to that eswitch.
Added:
o command to get statistics of eswitch and function.
o sysfs support to export eswitch and func statatistics.
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic.h | 33 ++++++++++
drivers/net/qlcnic/qlcnic_ctx.c | 125 ++++++++++++++++++++++++++++++++++++
drivers/net/qlcnic/qlcnic_main.c | 132 +++++++++++++++++++++++++++++++++++++-
3 files changed, 289 insertions(+), 1 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 9703893..ae02b53 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -555,6 +555,7 @@ struct qlcnic_recv_context {
#define QLCNIC_CDRP_CMD_GET_ESWITCH_STATUS 0x00000026
#define QLCNIC_CDRP_CMD_SET_PORTMIRRORING 0x00000027
#define QLCNIC_CDRP_CMD_CONFIGURE_ESWITCH 0x00000028
+#define QLCNIC_CDRP_CMD_GET_ESWITCH_STATS 0x0000002a
#define QLCNIC_RCODE_SUCCESS 0
#define QLCNIC_RCODE_TIMEOUT 17
@@ -1126,6 +1127,33 @@ struct qlcnic_esw_func_cfg {
u8 reserved;
};
+#define QLCNIC_STATS_VERSION 1
+#define QLCNIC_STATS_PORT 1
+#define QLCNIC_STATS_ESWITCH 2
+#define QLCNIC_QUERY_RX_COUNTER 0
+#define QLCNIC_QUERY_TX_COUNTER 1
+struct __qlcnic_esw_statistics {
+
+ __le16 context_id;
+ __le16 version;
+ __le16 size;
+ __le16 unused;
+
+ __le64 unicast_frames;
+ __le64 multicast_frames;
+ __le64 broadcast_frames;
+ __le64 dropped_frames;
+ __le64 errors;
+ __le64 local_frames;
+ __le64 numbytes;
+ __le64 rsvd[3];
+};
+
+struct qlcnic_esw_statistics {
+ struct __qlcnic_esw_statistics rx;
+ struct __qlcnic_esw_statistics tx;
+};
+
int qlcnic_fw_cmd_query_phy(struct qlcnic_adapter *adapter, u32 reg, u32 *val);
int qlcnic_fw_cmd_set_phy(struct qlcnic_adapter *adapter, u32 reg, u32 val);
@@ -1252,6 +1280,11 @@ int qlcnic_toggle_eswitch(struct qlcnic_adapter *, u8, u8);
int qlcnic_config_switch_port(struct qlcnic_adapter *, u8, int, u8, u8,
u8, u8, u16);
int qlcnic_config_port_mirroring(struct qlcnic_adapter *, u8, u8, u8);
+int qlcnic_get_port_stats(struct qlcnic_adapter *, const u8, const u8,
+ struct __qlcnic_esw_statistics *);
+int qlcnic_get_eswitch_stats(struct qlcnic_adapter *, const u8, u8,
+ struct __qlcnic_esw_statistics *);
+int qlcnic_clear_esw_stats(struct qlcnic_adapter *adapter, u8, u8, u8);
extern int qlcnic_config_tso;
/*
diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
index cc5d861..57c9b09 100644
--- a/drivers/net/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/qlcnic/qlcnic_ctx.c
@@ -983,3 +983,128 @@ int qlcnic_config_switch_port(struct qlcnic_adapter *adapter, u8 id,
return err;
}
+
+int qlcnic_get_port_stats(struct qlcnic_adapter *adapter, const u8 func,
+ const u8 rx_tx, struct __qlcnic_esw_statistics *esw_stats) {
+
+ size_t stats_size = sizeof(struct __qlcnic_esw_statistics);
+ dma_addr_t stats_dma_t;
+ void *stats_addr;
+ u32 arg1;
+ int err;
+
+ if (esw_stats == NULL)
+ return -ENOMEM;
+
+ if (adapter->op_mode != QLCNIC_MGMT_FUNC &&
+ func != adapter->ahw.pci_func) {
+ dev_err(&adapter->pdev->dev,
+ "Not privilege to query stats for func=%d", func);
+ return -EIO;
+ }
+
+ stats_addr = pci_alloc_consistent(adapter->pdev, stats_size,
+ &stats_dma_t);
+ if (!stats_addr) {
+ dev_err(&adapter->pdev->dev, "Unable to allocate memory\n");
+ return -ENOMEM;
+ }
+ memset(stats_addr, 0, stats_size);
+
+ arg1 = func | QLCNIC_STATS_VERSION << 8 | QLCNIC_STATS_PORT << 12;
+ arg1 |= rx_tx << 15 | stats_size << 16;
+
+ err = qlcnic_issue_cmd(adapter,
+ adapter->ahw.pci_func,
+ adapter->fw_hal_version,
+ arg1,
+ MSD(stats_dma_t),
+ LSD(stats_dma_t),
+ QLCNIC_CDRP_CMD_GET_ESWITCH_STATS);
+
+ if (!err)
+ memcpy(esw_stats, stats_addr, stats_size);
+
+ pci_free_consistent(adapter->pdev, stats_size, stats_addr,
+ stats_dma_t);
+ return err;
+}
+
+int qlcnic_get_eswitch_stats(struct qlcnic_adapter *adapter, const u8 eswitch,
+ const u8 rx_tx, struct __qlcnic_esw_statistics *esw_stats) {
+
+ struct __qlcnic_esw_statistics port_stats;
+ u8 i;
+ int ret = -EIO;
+
+ if (esw_stats == NULL)
+ return -ENOMEM;
+ if (adapter->op_mode != QLCNIC_MGMT_FUNC)
+ return -EIO;
+ if (adapter->npars == NULL)
+ return -EIO;
+
+ memset(esw_stats, 0, sizeof(struct __qlcnic_esw_statistics));
+ esw_stats->context_id = eswitch;
+
+ for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
+ if (adapter->npars[i].phy_port != eswitch)
+ continue;
+
+ memset(&port_stats, 0, sizeof(struct __qlcnic_esw_statistics));
+ if (qlcnic_get_port_stats(adapter, i, rx_tx, &port_stats))
+ continue;
+
+ esw_stats->size = port_stats.size;
+ esw_stats->version = port_stats.version;
+ esw_stats->unicast_frames += port_stats.unicast_frames;
+ esw_stats->multicast_frames += port_stats.multicast_frames;
+ esw_stats->broadcast_frames += port_stats.broadcast_frames;
+ esw_stats->dropped_frames += port_stats.dropped_frames;
+ esw_stats->errors += port_stats.errors;
+ esw_stats->local_frames += port_stats.local_frames;
+ esw_stats->numbytes += port_stats.numbytes;
+
+ ret = 0;
+ }
+ return ret;
+}
+
+int qlcnic_clear_esw_stats(struct qlcnic_adapter *adapter, const u8 func_esw,
+ const u8 port, const u8 rx_tx)
+{
+
+ u32 arg1;
+
+ if (adapter->op_mode != QLCNIC_MGMT_FUNC)
+ return -EIO;
+
+ if (func_esw == QLCNIC_STATS_PORT) {
+ if (port >= QLCNIC_MAX_PCI_FUNC)
+ goto err_ret;
+ } else if (func_esw == QLCNIC_STATS_ESWITCH) {
+ if (port >= QLCNIC_NIU_MAX_XG_PORTS)
+ goto err_ret;
+ } else {
+ goto err_ret;
+ }
+
+ if (rx_tx > QLCNIC_QUERY_TX_COUNTER)
+ goto err_ret;
+
+ arg1 = port | QLCNIC_STATS_VERSION << 8 | func_esw << 12;
+ arg1 |= BIT_14 | rx_tx << 15;
+
+ return qlcnic_issue_cmd(adapter,
+ adapter->ahw.pci_func,
+ adapter->fw_hal_version,
+ arg1,
+ 0,
+ 0,
+ QLCNIC_CDRP_CMD_GET_ESWITCH_STATS);
+
+err_ret:
+ dev_err(&adapter->pdev->dev, "Invalid argument func_esw=%d port=%d"
+ "rx_ctx=%d\n", func_esw, port, rx_tx);
+ return -EIO;
+}
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 49c8110..96e8e75 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -3368,6 +3368,115 @@ qlcnic_sysfs_read_npar_config(struct file *file, struct kobject *kobj,
}
static ssize_t
+qlcnic_sysfs_get_port_stats(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t offset, size_t size)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+ struct qlcnic_esw_statistics port_stats;
+ int ret;
+
+ if (size != sizeof(struct qlcnic_esw_statistics))
+ return QL_STATUS_INVALID_PARAM;
+
+ if (offset >= QLCNIC_MAX_PCI_FUNC)
+ return QL_STATUS_INVALID_PARAM;
+
+ memset(&port_stats, 0, size);
+ ret = qlcnic_get_port_stats(adapter, offset, QLCNIC_QUERY_RX_COUNTER,
+ &port_stats.rx);
+ if (ret)
+ return ret;
+
+ ret = qlcnic_get_port_stats(adapter, offset, QLCNIC_QUERY_TX_COUNTER,
+ &port_stats.tx);
+ if (ret)
+ return ret;
+
+ memcpy(buf, &port_stats, size);
+ return size;
+}
+
+static ssize_t
+qlcnic_sysfs_get_esw_stats(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t offset, size_t size)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+ struct qlcnic_esw_statistics esw_stats;
+ int ret;
+
+ if (size != sizeof(struct qlcnic_esw_statistics))
+ return QL_STATUS_INVALID_PARAM;
+
+ if (offset >= QLCNIC_NIU_MAX_XG_PORTS)
+ return QL_STATUS_INVALID_PARAM;
+
+ memset(&esw_stats, 0, size);
+ ret = qlcnic_get_eswitch_stats(adapter, offset, QLCNIC_QUERY_RX_COUNTER,
+ &esw_stats.rx);
+ if (ret)
+ return ret;
+
+ ret = qlcnic_get_eswitch_stats(adapter, offset, QLCNIC_QUERY_TX_COUNTER,
+ &esw_stats.tx);
+ if (ret)
+ return ret;
+
+ memcpy(buf, &esw_stats, size);
+ return size;
+}
+
+static ssize_t
+qlcnic_sysfs_clear_esw_stats(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t offset, size_t size)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+ int ret;
+
+ if (offset >= QLCNIC_NIU_MAX_XG_PORTS)
+ return QL_STATUS_INVALID_PARAM;
+
+ ret = qlcnic_clear_esw_stats(adapter, QLCNIC_STATS_ESWITCH, offset,
+ QLCNIC_QUERY_RX_COUNTER);
+ if (ret)
+ return ret;
+
+ ret = qlcnic_clear_esw_stats(adapter, QLCNIC_STATS_ESWITCH, offset,
+ QLCNIC_QUERY_TX_COUNTER);
+ if (ret)
+ return ret;
+
+ return size;
+}
+
+static ssize_t
+qlcnic_sysfs_clear_port_stats(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t offset, size_t size)
+{
+
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+ int ret;
+
+ if (offset >= QLCNIC_MAX_PCI_FUNC)
+ return QL_STATUS_INVALID_PARAM;
+
+ ret = qlcnic_clear_esw_stats(adapter, QLCNIC_STATS_PORT, offset,
+ QLCNIC_QUERY_RX_COUNTER);
+ if (ret)
+ return ret;
+
+ ret = qlcnic_clear_esw_stats(adapter, QLCNIC_STATS_PORT, offset,
+ QLCNIC_QUERY_TX_COUNTER);
+ if (ret)
+ return ret;
+
+ return size;
+}
+
+static ssize_t
qlcnic_sysfs_read_pci_config(struct file *file, struct kobject *kobj,
struct bin_attribute *attr, char *buf, loff_t offset, size_t size)
{
@@ -3410,6 +3519,20 @@ static struct bin_attribute bin_attr_pci_config = {
.write = NULL,
};
+static struct bin_attribute bin_attr_port_stats = {
+ .attr = {.name = "port_stats", .mode = (S_IRUGO | S_IWUSR)},
+ .size = 0,
+ .read = qlcnic_sysfs_get_port_stats,
+ .write = qlcnic_sysfs_clear_port_stats,
+};
+
+static struct bin_attribute bin_attr_esw_stats = {
+ .attr = {.name = "esw_stats", .mode = (S_IRUGO | S_IWUSR)},
+ .size = 0,
+ .read = qlcnic_sysfs_get_esw_stats,
+ .write = qlcnic_sysfs_clear_esw_stats,
+};
+
static struct bin_attribute bin_attr_esw_config = {
.attr = {.name = "esw_config", .mode = (S_IRUGO | S_IWUSR)},
.size = 0,
@@ -3449,6 +3572,9 @@ qlcnic_create_diag_entries(struct qlcnic_adapter *adapter)
{
struct device *dev = &adapter->pdev->dev;
+ if (device_create_bin_file(dev, &bin_attr_port_stats))
+ dev_info(dev, "failed to create port stats sysfs entry");
+
if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC)
return;
if (device_create_file(dev, &dev_attr_diag_mode))
@@ -3468,7 +3594,8 @@ qlcnic_create_diag_entries(struct qlcnic_adapter *adapter)
dev_info(dev, "failed to create esw config sysfs entry");
if (device_create_bin_file(dev, &bin_attr_pm_config))
dev_info(dev, "failed to create pm config sysfs entry");
-
+ if (device_create_bin_file(dev, &bin_attr_esw_stats))
+ dev_info(dev, "failed to create eswitch stats sysfs entry");
}
static void
@@ -3476,6 +3603,8 @@ qlcnic_remove_diag_entries(struct qlcnic_adapter *adapter)
{
struct device *dev = &adapter->pdev->dev;
+ device_remove_bin_file(dev, &bin_attr_port_stats);
+
if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC)
return;
device_remove_file(dev, &dev_attr_diag_mode);
@@ -3488,6 +3617,7 @@ qlcnic_remove_diag_entries(struct qlcnic_adapter *adapter)
device_remove_bin_file(dev, &bin_attr_npar_config);
device_remove_bin_file(dev, &bin_attr_esw_config);
device_remove_bin_file(dev, &bin_attr_pm_config);
+ device_remove_bin_file(dev, &bin_attr_esw_stats);
}
#ifdef CONFIG_INET
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 2/8] qlcnic: device state management fixes for virtual func
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
o NPAR state should be set to operationl by Mangement function only.
o NPAR state should be set to non operational before device reset.
o VF function should wait for NPAR state to be operational.
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_hdr.h | 5 +-
drivers/net/qlcnic/qlcnic_main.c | 80 +++++++++++++++++++++++---------------
2 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_hdr.h b/drivers/net/qlcnic/qlcnic_hdr.h
index 15fc320..bd346d9 100644
--- a/drivers/net/qlcnic/qlcnic_hdr.h
+++ b/drivers/net/qlcnic/qlcnic_hdr.h
@@ -718,8 +718,9 @@ enum {
#define QLCNIC_DEV_FAILED 0x6
#define QLCNIC_DEV_QUISCENT 0x7
-#define QLCNIC_DEV_NPAR_NOT_RDY 0
-#define QLCNIC_DEV_NPAR_RDY 1
+#define QLCNIC_DEV_NPAR_NON_OPER 0 /* NON Operational */
+#define QLCNIC_DEV_NPAR_OPER 1 /* NPAR Operational */
+#define QLCNIC_DEV_NPAR_OPER_TIMEO 30 /* Operational time out */
#define QLC_DEV_CHECK_ACTIVE(VAL, FN) ((VAL) &= (1 << (FN * 4)))
#define QLC_DEV_SET_REF_CNT(VAL, FN) ((VAL) |= (1 << (FN * 4)))
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 9cc8732..bac2371 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -2388,7 +2388,7 @@ qlcnic_fwinit_work(struct work_struct *work)
{
struct qlcnic_adapter *adapter = container_of(work,
struct qlcnic_adapter, fw_work.work);
- u32 dev_state = 0xf, npar_state;
+ u32 dev_state = 0xf;
if (qlcnic_api_lock(adapter))
goto err_ret;
@@ -2402,16 +2402,8 @@ qlcnic_fwinit_work(struct work_struct *work)
}
if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC) {
- npar_state = QLCRD32(adapter, QLCNIC_CRB_DEV_NPAR_STATE);
- if (npar_state == QLCNIC_DEV_NPAR_RDY) {
- qlcnic_api_unlock(adapter);
- goto wait_npar;
- } else {
- qlcnic_schedule_work(adapter, qlcnic_fwinit_work,
- FW_POLL_DELAY);
- qlcnic_api_unlock(adapter);
- return;
- }
+ qlcnic_api_unlock(adapter);
+ goto wait_npar;
}
if (adapter->fw_wait_cnt++ > adapter->reset_ack_timeo) {
@@ -2460,20 +2452,17 @@ wait_npar:
QLCDB(adapter, HW, "Func waiting: Device state=%u\n", dev_state);
switch (dev_state) {
- case QLCNIC_DEV_QUISCENT:
- case QLCNIC_DEV_NEED_QUISCENT:
- case QLCNIC_DEV_NEED_RESET:
- qlcnic_schedule_work(adapter,
- qlcnic_fwinit_work, FW_POLL_DELAY);
- return;
- case QLCNIC_DEV_FAILED:
- break;
-
- default:
+ case QLCNIC_DEV_READY:
if (!adapter->nic_ops->start_firmware(adapter)) {
qlcnic_schedule_work(adapter, qlcnic_attach_work, 0);
return;
}
+ case QLCNIC_DEV_FAILED:
+ break;
+ default:
+ qlcnic_schedule_work(adapter,
+ qlcnic_fwinit_work, FW_POLL_DELAY);
+ return;
}
err_ret:
@@ -2520,6 +2509,22 @@ err_ret:
}
+/*Transit NPAR state to NON Operational */
+static void
+qlcnic_set_npar_non_operational(struct qlcnic_adapter *adapter)
+{
+ u32 state;
+
+ state = QLCRD32(adapter, QLCNIC_CRB_DEV_NPAR_STATE);
+ if (state == QLCNIC_DEV_NPAR_NON_OPER)
+ return;
+
+ if (qlcnic_api_lock(adapter))
+ return;
+ QLCWR32(adapter, QLCNIC_CRB_DEV_NPAR_STATE, QLCNIC_DEV_NPAR_NON_OPER);
+ qlcnic_api_unlock(adapter);
+}
+
/*Transit to RESET state from READY state only */
static void
qlcnic_dev_request_reset(struct qlcnic_adapter *adapter)
@@ -2538,6 +2543,7 @@ qlcnic_dev_request_reset(struct qlcnic_adapter *adapter)
qlcnic_idc_debug_info(adapter, 0);
}
+ QLCWR32(adapter, QLCNIC_CRB_DEV_NPAR_STATE, QLCNIC_DEV_NPAR_NON_OPER);
qlcnic_api_unlock(adapter);
}
@@ -2545,21 +2551,14 @@ qlcnic_dev_request_reset(struct qlcnic_adapter *adapter)
static void
qlcnic_dev_set_npar_ready(struct qlcnic_adapter *adapter)
{
- u32 state;
-
if (!(adapter->flags & QLCNIC_ESWITCH_ENABLED) ||
- adapter->op_mode == QLCNIC_NON_PRIV_FUNC)
+ adapter->op_mode != QLCNIC_MGMT_FUNC)
return;
if (qlcnic_api_lock(adapter))
return;
- state = QLCRD32(adapter, QLCNIC_CRB_DEV_NPAR_STATE);
-
- if (state != QLCNIC_DEV_NPAR_RDY) {
- QLCWR32(adapter, QLCNIC_CRB_DEV_NPAR_STATE,
- QLCNIC_DEV_NPAR_RDY);
- QLCDB(adapter, DRV, "NPAR READY state set\n");
- }
+ QLCWR32(adapter, QLCNIC_CRB_DEV_NPAR_STATE, QLCNIC_DEV_NPAR_OPER);
+ QLCDB(adapter, DRV, "NPAR operational state set\n");
qlcnic_api_unlock(adapter);
}
@@ -2621,8 +2620,11 @@ qlcnic_check_health(struct qlcnic_adapter *adapter)
qlcnic_dev_request_reset(adapter);
state = QLCRD32(adapter, QLCNIC_CRB_DEV_STATE);
- if (state == QLCNIC_DEV_NEED_RESET || state == QLCNIC_DEV_NEED_QUISCENT)
+ if (state == QLCNIC_DEV_NEED_RESET ||
+ state == QLCNIC_DEV_NEED_QUISCENT) {
+ qlcnic_set_npar_non_operational(adapter);
adapter->need_fw_reset = 1;
+ }
heartbit = QLCRD32(adapter, QLCNIC_PEG_ALIVE_COUNTER);
if (heartbit != adapter->heartbit) {
@@ -2812,11 +2814,25 @@ static int
qlcnicvf_start_firmware(struct qlcnic_adapter *adapter)
{
int err;
+ u8 npar_opt_timeo = QLCNIC_DEV_NPAR_OPER_TIMEO;
+ u32 npar_state;
err = qlcnic_can_start_firmware(adapter);
if (err)
return err;
+ npar_state = QLCRD32(adapter, QLCNIC_CRB_DEV_NPAR_STATE);
+ while (npar_state != QLCNIC_DEV_NPAR_OPER && --npar_opt_timeo) {
+ msleep(1000);
+ npar_state = QLCRD32(adapter, QLCNIC_CRB_DEV_NPAR_STATE);
+ }
+
+ if (!npar_opt_timeo) {
+ dev_err(&adapter->pdev->dev,
+ "Waiting for NPAR state to opertional timeout\n");
+ return -EIO;
+ }
+
qlcnic_check_options(adapter);
adapter->need_fw_reset = 0;
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 1/8] qlcnic: fix aer for virtual func
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
Virtual function are not privilge to initialize firmware.
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index b9615bd..9cc8732 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -2723,7 +2723,7 @@ static int qlcnic_attach_func(struct pci_dev *pdev)
if (qlcnic_api_lock(adapter))
return -EINVAL;
- if (first_func) {
+ if (adapter->op_mode != QLCNIC_NON_PRIV_FUNC && first_func) {
adapter->need_fw_reset = 1;
set_bit(__QLCNIC_START_FW, &adapter->state);
QLCWR32(adapter, QLCNIC_CRB_DEV_STATE, QLCNIC_DEV_INITIALIZING);
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 7/8] qlcnic: turn off lro when rxcsum is disabled.
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic.h | 1 +
drivers/net/qlcnic/qlcnic_ethtool.c | 8 ++++++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index ae02b53..1d680a5 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -894,6 +894,7 @@ struct qlcnic_mac_req {
#define QLCNIC_MSI_ENABLED 0x02
#define QLCNIC_MSIX_ENABLED 0x04
#define QLCNIC_LRO_ENABLED 0x08
+#define QLCNIC_LRO_DISABLED 0x00
#define QLCNIC_BRIDGE_ENABLED 0X10
#define QLCNIC_DIAG_ENABLED 0x20
#define QLCNIC_ESWITCH_ENABLED 0x40
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 3d4655d..706eb80 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -819,6 +819,14 @@ static int qlcnic_set_rx_csum(struct net_device *dev, u32 data)
{
struct qlcnic_adapter *adapter = netdev_priv(dev);
adapter->rx_csum = !!data;
+
+ if (!(adapter->capabilities & QLCNIC_FW_CAPABILITY_HW_LRO))
+ return 0;
+
+ if (!adapter->rx_csum && !(dev->features & NETIF_F_LRO)) {
+ dev->features &= ~NETIF_F_LRO;
+ qlcnic_config_hw_lro(adapter, QLCNIC_LRO_DISABLED);
+ }
return 0;
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 8/8] qlcnic: update version 5.0.8.0
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
New field added to version number "_UPDATE",
to distinguish fixes and enhancement.
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 1d680a5..8e711f1 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -51,8 +51,9 @@
#define _QLCNIC_LINUX_MAJOR 5
#define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 7
-#define QLCNIC_LINUX_VERSIONID "5.0.7"
+#define _QLCNIC_LINUX_SUBVERSION 8
+#define _QLCNIC_LINUX_UPDATE 0
+#define QLCNIC_LINUX_VERSIONID "5.0.8.0"
#define QLCNIC_DRV_IDC_VER 0x01
#define QLCNIC_VERSION_CODE(a, b, c) (((a) << 24) + ((b) << 16) + (c))
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 5/8] qlcnic: fix link status message
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman, Sony Chacko
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
From: Sony Chacko <sony.chacko@qlogic.com>
Display interface name for link status message.
With dev_info pci number get printed.
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_main.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 96e8e75..7c9072d 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -1934,14 +1934,16 @@ void qlcnic_advert_link_change(struct qlcnic_adapter *adapter, int linkup)
struct net_device *netdev = adapter->netdev;
if (adapter->ahw.linkup && !linkup) {
- dev_info(&netdev->dev, "NIC Link is down\n");
+ printk(KERN_INFO "%s: %s NIC Link is down\n",
+ qlcnic_driver_name, netdev->name);
adapter->ahw.linkup = 0;
if (netif_running(netdev)) {
netif_carrier_off(netdev);
netif_stop_queue(netdev);
}
} else if (!adapter->ahw.linkup && linkup) {
- dev_info(&netdev->dev, "NIC Link is up\n");
+ printk(KERN_INFO "%s: %s NIC Link is up\n",
+ qlcnic_driver_name, netdev->name);
adapter->ahw.linkup = 1;
if (netif_running(netdev)) {
netif_carrier_on(netdev);
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 6/8] qlcnic: fix link diag test
From: Amit Kumar Salecha @ 2010-08-11 12:05 UTC (permalink / raw)
To: davem; +Cc: netdev, ameen.rahman, Sony Chacko
In-Reply-To: <1281528329-8812-1-git-send-email-amit.salecha@qlogic.com>
From: Sony Chacko <sony.chacko@qlogic.com>
o Give sufficient time for all makes of PHY modules
to establish the link after loopback test.
o Perform online test before offline test.
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_ethtool.c | 17 ++++++++---------
drivers/net/qlcnic/qlcnic_hw.c | 1 +
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 9328d59..3d4655d 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -747,6 +747,14 @@ qlcnic_diag_test(struct net_device *dev, struct ethtool_test *eth_test,
{
memset(data, 0, sizeof(u64) * QLCNIC_TEST_LEN);
+ data[0] = qlcnic_reg_test(dev);
+ if (data[0])
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+
+ data[1] = (u64) qlcnic_test_link(dev);
+ if (data[1])
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+
if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
data[2] = qlcnic_irq_test(dev);
if (data[2])
@@ -757,15 +765,6 @@ qlcnic_diag_test(struct net_device *dev, struct ethtool_test *eth_test,
eth_test->flags |= ETH_TEST_FL_FAILED;
}
-
- data[0] = qlcnic_reg_test(dev);
- if (data[0])
- eth_test->flags |= ETH_TEST_FL_FAILED;
-
- /* link test */
- data[1] = (u64) qlcnic_test_link(dev);
- if (data[1])
- eth_test->flags |= ETH_TEST_FL_FAILED;
}
static void
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index e08c8b0..9d40ce0 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -1245,4 +1245,5 @@ void qlcnic_clear_ilb_mode(struct qlcnic_adapter *adapter)
mode = VPORT_MISS_MODE_ACCEPT_MULTI;
qlcnic_nic_set_promisc(adapter, mode);
+ msleep(1000);
}
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH] netpoll: use non-BH variant of RCU
From: Paul E. McKenney @ 2010-08-11 12:20 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, linville, netdev
In-Reply-To: <20100811110330.GA6440@gondor.apana.org.au>
On Wed, Aug 11, 2010 at 07:03:30AM -0400, Herbert Xu wrote:
> On Tue, Aug 10, 2010 at 04:31:17PM -0700, David Miller wrote:
> >
> > Linus's tree is fine for this.
> >
> > But we need this fix for 2.6.35-stable too. Herbert would
> > you object to putting John's more simple fix there?
>
> To use rcu_read_lock safely we'd also need to add do synchronize_rcu
> in addition of synchronize_rcu_bh, right Paul?
That is true.
> Of course as we were doing this unsafely prior to my patch anyway
> I'm also fine with just reverting it.
That is true as well.
Thanx, Paul
^ permalink raw reply
* Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35
From: Wolfgang Grandegger @ 2010-08-11 12:31 UTC (permalink / raw)
To: Masayuki Ohtak
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
gregkh-l3A5Bk7waGM, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
meego-dev-WXzIur8shnEAvxtiuMwx3w, arjan-VuQAYsv1563Yd54FQh9/CA
In-Reply-To: <4C61EDE5.4030505-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Hello,
On 08/11/2010 02:25 AM, Masayuki Ohtak wrote:
> CAN driver of Topcliff PCH
>
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus.
That's interesting. Where can I/we find more information about this CAN
controller, e.g. data-sheets. It seems to have a few interesting
features (message scheduler, etc.).
> Topcliff PCH has CAN I/F. This driver enables CAN function.
>
> Signed-off-by: Masayuki Ohtake <masa-korg-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Thanks for your contribution. Unfortunately, there are many issues,
especially the driver is not yet conform with the Socket-CAN driver API:
- My first observation was:
$ wc -l pch_can.c
4076 pch_can.c
$ grep dev_dbg pch_can.c | wc -l
143
That's a lot of code, mainly debugging code, I think. This needs to
be cleaned up sooner than later. dev_dbg's should be restricted to a
few useful for the real users.
- The values for the hw-specific bit-timing registers should be derived
from the calculated values in "priv->can.bittiming":
http://lxr.linux.no/#linux+v2.6.35/include/linux/can/netlink.h#L17
- The driver should handle state changes and communicate them to the
user space via error messages, if possible.
- The driver should report errors to the user space via error messages.
- Bus errors seem not to be handled properly.I'm missing can_bus_off().
Does the controller recover from bus-off automatically?
- I see that the driver uses many TX and RX objects. How do you avoid
out-of-order transmission and reception?
- Various CAN controller modes, like listen_only and loopback can be
handled via "priv->can.ctrlmode". Please use that interface if
appropriate.
- Please use a structure to describe the register layout, instead of
defines to profit from type checking.
- As you are at it, please also fix coding style issues, especially for
comments as described here:
http://lxr.linux.no/#linux+v2.6.35/Documentation/CodingStyle#L425
You can take the SJA1000 driver as example. Further useful information
is here:
http://lxr.linux.no/#linux+v2.6.35/Documentation/networking/can.txt
http://svn.berlios.de/wsvn/socketcan/trunk/README.submitting-patches
It follows a quick review. It's to early for a detailed one:
...
> +/* Array to store the timing settings. */
> +static struct pch_can_timing can_rec_timing[] = {
> + /* <Baud rate> <BRP> <TS1> <TS2> <SJW> */
> + /* settings for 62.5MHz */
> + {0xa, 0x250, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 10 kbits/s */
> + {0x14, 0x8D, 0xB, 0x5, 0x0, 0x0, 0x0}, /* < 20 kbits/s */
> + {0x32, 0x5C, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 50 kbits/s */
> + {0x7d, 0x18, 0xC, 0x5, 0x0, 0x0, 0x0}, /* < 125 kbits/s */
> + {0xfa, 0x18, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 250 kbits/s */
> + {0x1f4, 0x8, 0x9, 0x2, 0x0, 0x0, 0x0}, /* < 500 kbits/s */
> + {0x320, 0x5, 0x8, 0x2, 0x0, 0x0, 0x0}, /* < 800 kbits/s */
> + {0x3e8, 0x2, 0xC, 0x6, 0x0, 0x0, 0x0}, /* < 1000 kbits/s */
> +
> + /* settings for 24MHz */
> + {0xa, 0xCF, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 10 kbits/s */
> + {0x14, 0x57, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 20 kbits/s */
> + {0x32, 0xF, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 50 kbits/s */
> + {0x7d, 0xF, 0x8, 0x1, 0x0, 0x0, 0x0}, /* < 125 kbits/s */
> + {0xfa, 0x7, 0x8, 0x1, 0x0, 0x0, 0x0}, /* < 250 kbits/s */
> + {0x1f4, 0x3, 0x8, 0x1, 0x0, 0x0, 0x0}, /* < 500 kbits/s */
> + {0x320, 0x2, 0x7, 0x0, 0x0, 0x0, 0x0}, /* < 800 kbits/s */
> + {0x3e8, 0x1, 0x8, 0x1, 0x0, 0x0, 0x0}, /* < 1000 kbits/s */
> +
> + /* settings for 50MHz */
> + {0xa, 0xFA, 0xC, 0x5, 0x1, 0x0, 0x0}, /* < 10 kbits/s */
> + {0x14, 0x7D, 0xC, 0x5, 0x1, 0x0, 0x0}, /* < 20 kbits/s */
> + {0x32, 0x32, 0xF, 0x2, 0x0, 0x0, 0x0}, /* < 50 kbits/s */
> + {0x7d, 0x19, 0xC, 0x1, 0x0, 0x0, 0x0}, /* < 125 kbits/s */
> + {0xfa, 0xA, 0xF, 0x2, 0x0, 0x0, 0x0}, /* < 250 kbits/s */
> + {0x1f4, 0x5, 0xF, 0x2, 0x0, 0x0, 0x0}, /* < 500 kbits/s */
> + {0x320, 0x5, 0x8, 0x2, 0x1, 0x0, 0x0}, /* < 800 kbits/s */
> + {0x3e8, 0x2, 0xF, 0x7, 0x0, 0x0, 0x0} /* < 1000 kbits/s */
> + /* Add the new clock settings here. */
> +};
Can't the register values be determined from the calculated one in
"priv->can.bittiming"? See comment above. Be aware the the user might
want to set custom values as described here:
http://lxr.linux.no/#linux+v2.6.35/Documentation/networking/can.txt#L730
> +
> +static DEFINE_MUTEX(pch_can_mutex);
What is this mutex good for. At a first glance, I don't think it's needed.
> +
> +#ifdef PCH_CAN_FIFO_MODE
The functions above are not used anywhere! Dead code? Please clean up.
> +static int check_can_fifo_status(int handle)
> +{
> + int ret_val;
> + struct can_fifo *f = (struct can_fifo *) handle;
> +
> + if (f->head == f->tail)
> + ret_val = PCH_CAN_FIFO_EMPTY;
> + else if (f->head->next == f->tail)
> + ret_val = PCH_CAN_FIFO_FULL;
> + else
> + ret_val = PCH_CAN_FIFO_NOT_EMPTY;
> +
> + return ret_val;
> +}
> +
...
> +static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> + int err; /* error variable. */
> + int ret;
> + struct pch_can_msg msg; /* The message object for writing. */
> + struct pch_can_priv *priv = netdev_priv(ndev);
> + struct pch_can_os *can_os = priv->pch_can_os_p;
> + struct can_frame *canframe_dat = (struct can_frame *)skb->data;
> + struct net_device_stats *stats = &ndev->stats;
> +
> + ret = mutex_lock_interruptible(&pch_can_mutex);
> + if (ret)
> + return -ERESTARTSYS;
This is an invalid return code.
> +
> + /* Translate CAN core format to CAN PCH's HW format */
> + memset(&msg, 0, sizeof(msg));
> + msg.ide = canframe_dat->can_id & 0x80000000;
> + if (canframe_dat->can_id & 0x80000000) {
> + msg.ide = 1;
> + msg.id = canframe_dat->can_id & 0x1fffffff;/* Extended
> + Message */
> + } else {
> + msg.ide = 0;
> + msg.id = canframe_dat->can_id & 0x00000fff;/* Standard
> + Message */
> +
> + }
> +
> + msg.dlc = canframe_dat->can_dlc;
> + memcpy(&msg.data, canframe_dat->data, 8);
> +
> + if (canframe_dat->can_id & 0x40000000)
> + msg.rtr = 1;
> + else
> + msg.rtr = 0;
> +
> + /* If device suspended. */
> + if ((can_os->is_suspending) == 1) {
> + dev_err(&ndev->dev,
> + "%s -> Device is in suspend mode.\n", __func__);
> + dev_dbg(&ndev->dev, "%s returns %d\n", __func__, -EAGAIN);
> + err = -EAGAIN;
> + goto err_out;
> + }
> +
> + can_put_echo_skb(skb, ndev, 0);
This will not work, as you are using more than one TX object.
> +MODULE_DESCRIPTION("Controller Area Network Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("0.94");
> +module_param_named(pch_can_rx_buf_size, pch_can_rx_buf_size, int, 444);
> +module_param_named(pch_can_tx_buf_size, pch_can_tx_buf_size, int, 444);
> +module_param_named(pch_can_clock, pch_can_clock, int, 444);
> +MODULE_DEVICE_TABLE(pci, pch_can_pcidev_id);
Please move these calls up to the beginning where the variables are
defined and provide a proper description.
> +
> +module_init(pch_can_pci_init);
> +module_exit(pch_can_pci_exit);
> diff --git a/drivers/net/can/pch_can.h b/drivers/net/can/pch_can.h
> new file mode 100644
> index 0000000..88a9559
> --- /dev/null
> +++ b/drivers/net/can/pch_can.h
...
> +/**
> + * struct pch_can_msg - CAN message structure
> + * @ide: Standard/extended msg
> + * @id: 11 or 29 bit msg id
> + * @dlc: Size of data
> + * @data: Message pay load
> + * @rtr: RTR message
> + */
> +struct pch_can_msg {
> + unsigned short ide;
> + unsigned int id;
> + unsigned short dlc;
> + unsigned char data[PCH_CAN_MSG_DATA_LEN];
> + unsigned short rtr;
> +};
Hm, why you can't use "struct can_frame".
> +
> +/**
> + * pch_can_timing - CAN bittiming structure
> + * @bitrate: Bitrate (kbps)
> + * @cfg_bitrate: Bitrate
> + * @cfg_tseg1: Tseg1
> + * @cfg_tseg2: Tseg2
> + * @cfg_sjw: Sync jump width
> + * @smpl_mode: Sampling mode
> + * @edge_mode: Edge R / D
> + */
> +struct pch_can_timing {
> + unsigned int bitrate;
> + unsigned int cfg_bitrate;
> + unsigned int cfg_tseg1;
> + unsigned int cfg_tseg2;
> + unsigned int cfg_sjw;
> + unsigned int smpl_mode;
> + unsigned int edge_mode;
> +};
Ditto.
> +/**
> + * struct pch_can_error - CAN error structure
> + * @rxgte96: Rx err cnt >=96
> + * @txgte96: Tx err cnt >=96
> + * @error_stat: Error state of CAN node,
> + * 00=error active (normal)
> + * 01=error passive
> + * 1x=bus off
> + * @rx_err_cnt: Rx error count
> + * @tx_err_cnt: Tx error count
> + */
> +struct pch_can_error {
> + unsigned int rxgte96;
> + unsigned int txgte96;
> + unsigned int error_stat;
> + unsigned int rx_err_cnt;
> + unsigned int tx_err_cnt;
> +};
> +
> +/**
> + * struct pch_can_acc_filter - CAN Filter structure
> + * @id: The id/mask data
> + * @id_ext: Standard/extended ID
> + * @rtr: RTR message
> + */
> +struct pch_can_acc_filter {
> + unsigned int id;
> + unsigned int id_ext;
> + unsigned int rtr;
> +};
> +
> +/**
> + * struct pch_can_rx_filter - CAN RX filter
> + * @num: Filter number
> + * @umask: UMask value
> + * @amr: Acceptance Mask Reg
> + * @aidr: Acceptance Control Reg
> + */
> +struct pch_can_rx_filter {
> + unsigned int num;
> + unsigned int umask;
> + struct pch_can_acc_filter amr;
> + struct pch_can_acc_filter aidr;
> +};
> +
> +/**
> + * struct pch_can_os - structure to store the CAN device information.
> + * @can: CAN: device handle
> + * @opened: Linux opened device
> + * @can_num: Linux: CAN Number
> + * @pci_remap: Linux: MMap regs
> + * @dev: Linux: PCI Device
> + * @irq: Linux: IRQ
> + * @block_mode: Blocking / non-blocking
> + * @rx_fifo: Rx FIFO
> + * @read_wait_queue: Linux: Read wait queue
> + * @write_wait_queue: Linux: Write wait queue
> + * @write_wait_flag: Linux: Write wait flag
> + * @read_wait_flag: Linux: Read wait flag
> + * @open_spinlock: Linux: Open lock variable
> + * @is_suspending: Linux: Is suspending state
> + * @inode: Linux: inode
> + * @timing: CAN: timing
> + * @run_mode: CAN: run mode
> + * @listen_mode: CAN: listen mode
> + * @arbiter_mode: CAN: arbiter mode
> + * @tx_enable: CAN: Tx buffer state
> + * @rx_enable: CAN: Rx buffer state
> + * @rx_link: CAN: Rx link set
> + * @int_enables: CAN: ints enabled
> + * @int_stat: CAN: int status
> + * @bus_off_interrupt: CAN: Buss off int flag
> + * @rx_filter: CAN: Rx filters
> + * @can_callback: CAN: callback function pointer
> + * @ndev: net_device pointer
> + * @tx_spinlock: CAN: transmission lock variable
> + */
> +struct pch_can_os {
> + int can;
> + unsigned int opened;
> + unsigned int can_num;
> + void __iomem *pci_remap;
> + struct pci_dev *dev;
> + unsigned int irq;
> + int block_mode;
> + int rx_fifo;
> + wait_queue_head_t read_wait_queue;
> + wait_queue_head_t write_wait_queue;
> + unsigned int write_wait_flag;
> + unsigned int read_wait_flag;
> + spinlock_t open_spinlock;
> + unsigned int is_suspending;
> + struct inode *inode;
> + struct pch_can_timing timing;
> + enum pch_can_run_mode run_mode;
> + enum pch_can_listen_mode listen_mode;
> + enum pch_can_arbiter arbiter_mode;
> + unsigned int tx_enable[MAX_MSG_OBJ];
> + unsigned int rx_enable[MAX_MSG_OBJ];
> + unsigned int rx_link[MAX_MSG_OBJ];
> + unsigned int int_enables;
> + unsigned int int_stat;
> + unsigned int bus_off_interrupt;
> + struct pch_can_rx_filter rx_filter[MAX_MSG_OBJ];
> + void (*can_callback) (struct pch_can_os *);
> + struct net_device *ndev;
> + spinlock_t tx_spinlock;
> +};
> +
> +/**
> + * struct pch_can_priv - CAN driver private data structure
> + * @can: MUST be first member/field
> + * @ndev: Pointer to net_device structure
> + * @clk: unused
> + * @base: Base address
> + * @scc_ram_offset: unused
> + * @hecc_ram_offset: unused
> + * @mbx_offset: unused
> + * @int_line: unused
> + * @mbx_lock: unused
> + * @tx_head: unused
> + * @tx_tail: unused
> + * @rx_next: unused
Hm, if it's not used, what is it then good for? I stop reviewing here.
It seems that you ported an existing driver to Linux!? I'm looking
forward for an optimized and efficient solution.
Thanks,
Wolfgang.
^ permalink raw reply
* Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35
From: Marc Kleine-Budde @ 2010-08-11 13:04 UTC (permalink / raw)
To: Masayuki Ohtak
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
gregkh-l3A5Bk7waGM, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
meego-dev-WXzIur8shnEAvxtiuMwx3w, arjan-VuQAYsv1563Yd54FQh9/CA,
Wolfgang Grandegger
In-Reply-To: <4C61EDE5.4030505-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2538 bytes --]
Masayuki Ohtak wrote:
> CAN driver of Topcliff PCH
>
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus.
> Topcliff PCH has CAN I/F. This driver enables CAN function.
Thanks for your contribution. Some remarks in addition to Wolfgang's.
- Try to send patches directly with git send-email
- Rebase your tree to net-next-2.6.
- don't use global variables
- don't use that "int handle", e.g.:
> static int pch_can_msg_tx(int handle, struct pch_can_msg *msg,
> struct net_device *ndev)
> {
> u32 id1 = 0;
> u32 id2 = 0;
> u32 data_a1 = 0;
> u32 data_a2 = 0;
> u32 data_b1 = 0;
> u32 data_b2 = 0;
> u32 tx_disable_counter = 0;
> u32 buffer_status = 0;
> u32 tx_buffer_avail = 0;
> u32 status;
> u32 i;
> u32 counter;
> enum pch_can_run_mode run_mode;
> int retval = 0;
> u32 if1_creq;
>
> if ((handle == 0) || (msg == NULL)) {
> dev_err(&ndev->dev, "%s -> Invalid Parameter.\n", __func__);
> retval = -EPERM;
> }
>
> else {
> /* Obatining the remap address for access. */
> struct can_hw *can = (struct can_hw *) handle;
>
use a proper struct. There are numerous drawbacks, no type safety it's
not 64 safe, bad style,...
- there are several checks for handle against 0 that make no real sense
- clean up you error paths:
if (error) {
/* error handling */
} else {
/* do real work */
}
This leads to a big indention level in the interesting code path, making
it very hard to read. Please rework your code this way:
if (error) {
/* error handling */
return error;
/* or */
goto out_handle_error;
}
/* do real work */
- get rid of the intermediate struct pch_can_msg:
Your data path is:
struct can_frame -> struct pch_can_msg -> registers
write from struct can_frame into registers directly
- what's the purpose of "p_can_os->can_callback", call the function
directly from the interrupt handler
- implement NAPI
- get rid of "1 << BIT_SHIFT_SIX" and friend,
use "1 << 6" or "BIT(6)" if you like defines
- use defines to set bits in struct can_frame can_id
cheers, 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: 260 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: QoS weirdness : HTB accuracy
From: Andrew Beverley @ 2010-08-11 14:27 UTC (permalink / raw)
To: hawk; +Cc: Julien Vehent, Philip A. Prindeville, Netdev, netfilter
In-Reply-To: <Pine.LNX.4.64.1007071457020.6631@ask.diku.dk>
On Wed, 2010-07-07 at 15:07 +0200, Jesper Dangaard Brouer wrote:
> On Sun, 4 Jul 2010, Andrew Beverley wrote:
>
> >>> I was, in fact, an error in my ruleset. I had put the 'linklayer atm' at
> >>> both the branch and leaf levels, so the overhead was computed twice,
> >>> creating those holes in the bandwidth.
> >>
> >> I am seeing similar behaviour with my setup. Am I making the same
> >> mistake? A subset of my rules is as follows:
> >>
> >> tc qdisc add dev ppp0 root handle 1: htb r2q 1
> >>
> >> tc class add dev ppp0 parent 1: classid 1:1 htb \
> >> rate ${DOWNLINK}kbit ceil ${DOWNLINK}kbit \
> >> overhead $overhead linklayer atm <------- Here
> >>
> >> tc class add dev ppp0 parent 1:1 classid 1:10 htb \
> >> rate 612kbit ceil 612kbit prio 0 \
> >> overhead $overhead linklayer atm <------- And here
> >>
> >> tc qdisc add dev ppp0 parent 1:10 handle 4210: \
> >> sfq perturb 10 limit 50
> >>
> >> tc filter add dev ppp0 parent 1:0 protocol ip \
> >> prio 10 handle 10 fw flowid 1:10
> >
> > I removed the overhead option on the first leaf, and the speeds change
> > to what I expect. However, the rules above are taken straight from the
> > ADSL Optimizer project, which was the source of the original overhead
> > patch for tc. So is the ADSL Optimizer project wrong?
>
> After looking at the HTB kernel code I believe that the ADSL Optimizer
> project is NOT wrong. You should/must set the linklayer option on both
> the root class and leaf (else you would be charging the root/parent node
> too little).
>
> It is the expected behavior that small packets cause a significant
> reduction in the available bandwidth on the ATM link. Small packets will
> (almost) always cause 2 ATM packets (being send) using 106 bytes, thus
> eg. sending a 40 bytes TCP ACK packet result in approx 62% overhead.
[sorry for the late reply]
Maybe I am using the linklayer atm option wrong:
1. I am using it with a PCI ADSL modem. As the kernel "knows" that it's
dealing with an ATM device, is there any need to even specify the option
in the first place?
2. My methodology for calculating the overall speed limit for the device
is to download several large files and see what throughput I get. I then
use this figure in the rules above. However, as soon as I add a
linklayer option, the maximum throughout of the device obviously drops,
but means that I never get the throughput that I know the line is
capable of. Assuming that I should even be using the linklayer atm
option, should I be specifying the line speed higher than that which I
physically observe during line speed tests?
For example, if I observe an ADSL line speed of 4000kbit/s, I then
specify 3600kbit/s for the root of the HTB class, but with the linklayer
atm option I only get about 3200kbit/s. Should I be specifying a higher
value in order to get the most out of the connection?
Andy
^ permalink raw reply
* [patch net-next-2.6] bridge: allow hub-like behaviour
From: Jiri Pirko @ 2010-08-11 14:40 UTC (permalink / raw)
To: netdev; +Cc: shemminger, bridge, davem
This patch allows bridge to behave much like ordinary hub. That means that
every frame received is forwarded to all ports. This functionality is of course
disabled by default, can be enabled via sysfs. This could be handy mainly for
testing purposes.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
net/bridge/br_device.c | 14 +++++++++-----
net/bridge/br_input.c | 12 +++++++++---
net/bridge/br_private.h | 1 +
net/bridge/br_sysfs_br.c | 25 +++++++++++++++++++++++++
4 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index cf09fe5..1bfc1c5 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -60,14 +60,18 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
}
mdst = br_mdb_get(br, skb);
- if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb))
+ if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
+ likely(!br->hub_mode))
br_multicast_deliver(mdst, skb);
else
br_flood_deliver(br, skb);
- } else if ((dst = __br_fdb_get(br, dest)) != NULL)
- br_deliver(dst->dst, skb);
- else
- br_flood_deliver(br, skb);
+ } else {
+ dst = __br_fdb_get(br, dest);
+ if (dst && likely(!br->hub_mode))
+ br_deliver(dst->dst, skb);
+ else
+ br_flood_deliver(br, skb);
+ }
out:
rcu_read_unlock();
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 826cd52..24ee87e 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -75,7 +75,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
if (is_multicast_ether_addr(dest)) {
mdst = br_mdb_get(br, skb);
- if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
+ if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
+ likely(!br->hub_mode)) {
if ((mdst && !hlist_unhashed(&mdst->mglist)) ||
br_multicast_is_router(br))
skb2 = skb;
@@ -89,8 +90,13 @@ int br_handle_frame_finish(struct sk_buff *skb)
br->dev->stats.multicast++;
} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
skb2 = skb;
- /* Do not forward the packet since it's local. */
- skb = NULL;
+ if (likely(!br->hub_mode)) {
+ /* Do not forward the packet since it's local. */
+ skb = NULL;
+ } else {
+ /* In hub mode we want to forward the packet to all. */
+ dst = NULL;
+ }
}
if (skb) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 75c90ed..b6a9a7e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -194,6 +194,7 @@ struct net_bridge
unsigned long ageing_time;
unsigned long bridge_hello_time;
unsigned long bridge_forward_delay;
+ bool hub_mode;
u8 group_addr[ETH_ALEN];
u16 root_port;
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 5c1e555..ac98aa4 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -345,6 +345,30 @@ static ssize_t store_flush(struct device *d,
}
static DEVICE_ATTR(flush, S_IWUSR, NULL, store_flush);
+static ssize_t show_hub_mode(struct device *d,
+ struct device_attribute *attr, char *buf)
+{
+ struct net_bridge *br = to_bridge(d);
+
+ return sprintf(buf, "%d\n", br->hub_mode ? 1 : 0);
+}
+
+static int set_hub_mode(struct net_bridge *br, unsigned long val)
+{
+ br->hub_mode = val ? true : false;
+ return 0;
+}
+
+static ssize_t store_hub_mode(struct device *d,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return store_bridge_parm(d, buf, len, set_hub_mode);
+}
+
+static DEVICE_ATTR(hub_mode, S_IRUGO | S_IWUSR,
+ show_hub_mode, store_hub_mode);
+
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
static ssize_t show_multicast_router(struct device *d,
struct device_attribute *attr, char *buf)
@@ -698,6 +722,7 @@ static struct attribute *bridge_attrs[] = {
&dev_attr_gc_timer.attr,
&dev_attr_group_addr.attr,
&dev_attr_flush.attr,
+ &dev_attr_hub_mode.attr,
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
&dev_attr_multicast_router.attr,
&dev_attr_multicast_snooping.attr,
--
1.7.2.1
^ permalink raw reply related
* Re: Problem with non aligned DMA in usbnet on ARM
From: Greg KH @ 2010-08-11 15:04 UTC (permalink / raw)
To: Martin Fuzzey
Cc: Russell King - ARM Linux, linux-usb, netdev, linux-arm-kernel
In-Reply-To: <AANLkTimX46QKAGhiyo8AodaXVjuzoxr5-5z2V4gRr1Vu@mail.gmail.com>
On Wed, Aug 11, 2010 at 12:11:52PM +0200, Martin Fuzzey wrote:
> > You haven't said what HCD you're using.
> >
> imx21_hcd (submitted by me and mainlined in 2.6.34)
>
> > In that case, I'd say the HCD must handle misaligned URBs itself so
> > behaviour is consistent between all implementations on different
> > platforms.
> >
> Here is a pointer to the thread where it was stated that HCD's don't
> have to handle this.
>
> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
No, that thread is about stack vs. heap allocations, not about alignment
issues.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] Fix deadlock between boomerang_interrupt and boomerang_start_tx in 3c59x
From: Neil Horman @ 2010-08-11 15:12 UTC (permalink / raw)
To: netdev
If netconsole is in use, there is a possibility for deadlock in 3c59x between
boomerang_interrupt and boomerang_start_xmit. Both routines take the vp->lock,
and if netconsole is in use, a pr_* call from the boomerang_interrupt routine
will result in the netconsole code attempting to trnasmit an skb, which can try
to take the same spin lock, resulting in deadlock.
The fix is pretty straightforward. This patch allocats a bit in the 3c59x
private structure to indicate that its handling an interrupt. If we get into
the transmit routine and that bit is set, we can be sure that we have recursed
and will deadlock if we continue, so instead we just return NETDEV_TX_BUSY, so
the stack requeues the skb to try again later.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/3c59x.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index c754d88..c685a55 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -633,7 +633,8 @@ struct vortex_private {
open:1,
medialock:1,
must_free_region:1, /* Flag: if zero, Cardbus owns the I/O region */
- large_frames:1; /* accept large frames */
+ large_frames:1, /* accept large frames */
+ handling_irq:1; /* private in_irq indicator */
int drv_flags;
u16 status_enable;
u16 intr_enable;
@@ -2133,6 +2134,15 @@ boomerang_start_xmit(struct sk_buff *skb, struct net_device *dev)
dev->name, vp->cur_tx);
}
+ /*
+ * We can't allow a recursion from our interrupt handler back into the
+ * tx routine, as they take the same spin lock, and that causes
+ * deadlock. Just return NETDEV_TX_BUSY and let the stack try again in
+ * a bit
+ */
+ if (vp->handling_irq)
+ return NETDEV_TX_BUSY;
+
if (vp->cur_tx - vp->dirty_tx >= TX_RING_SIZE) {
if (vortex_debug > 0)
pr_warning("%s: BUG! Tx Ring full, refusing to send buffer.\n",
@@ -2335,11 +2345,13 @@ boomerang_interrupt(int irq, void *dev_id)
ioaddr = vp->ioaddr;
+
/*
* It seems dopey to put the spinlock this early, but we could race against vortex_tx_timeout
* and boomerang_start_xmit
*/
spin_lock(&vp->lock);
+ vp->handling_irq = 1;
status = ioread16(ioaddr + EL3_STATUS);
@@ -2447,6 +2459,7 @@ boomerang_interrupt(int irq, void *dev_id)
pr_debug("%s: exiting interrupt, status %4.4x.\n",
dev->name, status);
handler_exit:
+ vp->handling_irq = 0;
spin_unlock(&vp->lock);
return IRQ_HANDLED;
}
^ permalink raw reply related
* Re: [PATCH] netpoll: use non-BH variant of RCU
From: John W. Linville @ 2010-08-11 15:37 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, paulmck, netdev
In-Reply-To: <20100811112706.GA6772@gondor.apana.org.au>
On Wed, Aug 11, 2010 at 07:27:06AM -0400, Herbert Xu wrote:
> On Wed, Aug 11, 2010 at 07:03:30AM -0400, Herbert Xu wrote:
> >
> > To use rcu_read_lock safely we'd also need to add do synchronize_rcu
> > in addition of synchronize_rcu_bh, right Paul?
> >
> > Of course as we were doing this unsafely prior to my patch anyway
> > I'm also fine with just reverting it.
>
> Actually, we could just disable IRQs for stable. How about this
> patch (untested)?
>
> netpoll: Disable IRQ around RCU dereference in netpoll_rx
>
> We cannot use rcu_dereference_bh safely in netpoll_rx as we may
> be called with IRQs disabled. We could however simply disable
> IRQs as that too causes BH to be disabled and is safe in either
> case.
>
> Thanks to John Linville for discovering this bug and providing
> a patch.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This also seems to work for me.
John
P.S. As pointed-out elsewhere, my patch neglected to change
synchronize_rcu_bh to synchronize_rcu in netpoll.c. If some fault
is found w/ Herbert's patch then I can post a modified version of mine.
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [patch net-next-2.6] bridge: allow hub-like behaviour
From: Stephen Hemminger @ 2010-08-11 16:07 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <20100811144017.GA2625@psychotron.brq.redhat.com>
NO. unnecessary
Already possible, without patch. Just set the hold time
on the bridge to 0!
^ permalink raw reply
* Re: Problem with non aligned DMA in usbnet on ARM
From: Martin Fuzzey @ 2010-08-11 16:08 UTC (permalink / raw)
To: Greg KH; +Cc: Russell King - ARM Linux, linux-usb, netdev, linux-arm-kernel
In-Reply-To: <20100811150443.GB8867@kroah.com>
On Wed, Aug 11, 2010 at 5:04 PM, Greg KH <greg@kroah.com> wrote:
>> Here is a pointer to the thread where it was stated that HCD's don't
>> have to handle this.
>>
>> http://kerneltrap.org/mailarchive/linux-usb/2009/4/20/5528164
>
> No, that thread is about stack vs. heap allocations, not about alignment
> issues.
>
Well although the issue discussed in that thread was caused by a stack
allocation isn't the issue here the same?
My understanding is that a heap allocation as returned by kmalloc() will be:
1) correctly aligned for DMA
and
2) in a memory zone accessible to DMA
whereas a stack allocation is not guaranteed to have either of these properties.
The problem I described in that thread was due to case 1
(misalignment) rather than the stack memory zone not being accessible
at all to DMA.
To which was the reply was basically "use a heap allocation".
So the question is are hcds expected to accept arbitarilly aligned but
heap allocated pointers (such as the result of kmalloc() + 1)?
regards,
Martin
> thanks,
>
> greg k-h
>
^ 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