* Re: [net-next-2.6 v2] can: add support for CAN interface cards based on the PLX90xx PCI bridge
From: Wolfgang Grandegger @ 2010-02-02 8:02 UTC (permalink / raw)
To: Ira W. Snyder
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100201210507.GD32624-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
Ira W. Snyder wrote:
> On Mon, Feb 01, 2010 at 09:31:06PM +0100, Wolfgang Grandegger wrote:
>> Ira W. Snyder wrote:
>>> On Tue, Feb 02, 2010 at 01:42:44AM +0600, Pavel B. Cheblakov wrote:
>>>> From: Pavel Cheblakov <P.B.Cheblakov-tHBF8D5G73F4OK5fxMSSsQ@public.gmane.org>
>>>>
>>>> This driver is for CAN interface cards based on the PLX90xx PCI bridge.
>>>> Driver supports now:
>>>> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/)
>>>> - Adlink PCI-7841/cPCI-7841 SE card
>>>> - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
>>>> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
>>>>
>>>> Changes since v1:
>>>> - Added some defines for static inline int plx_pci_check_sja1000(...)
>>>> - static struct pci_device_id plx_pci_tbl[] replaced by
>>>> static DEFINE_PCI_DEVICE_TABLE(plx_pci_tbl)
>>>> - Typo fixed
>>>>
>>> Hello,
>>>
>>> Any ideas if this driver could support the Janz PCI CAN cards as well? I
>>> don't have any documentation for the cards, and I'd love to get away
>>> from their closed-source drivers.
>> Is it a passive or and intelligent CAN PCI card? What is the name of the
>> card? A intelligent card will run its own firmware and you do not have
>> direct access to the SJA1000. Pavel plx_pci driver is quite generic and
>> it should not be a big deal to support a PLX90xx based passive card.
>>
>
> It is an intelligent CAN PCI card. I'm pretty sure they're the card
> described here: http://www.janz.de/as/en/can-cp6/k2.html
>
> Since this driver is for passive cards, it clearly won't work. Are there
> any examples of drivers written for intelligent CAN cards? Until I found
> the website above, I'd never seen any data sheets for this card. They
> look like they may have enough information to write a driver to
> interface with the new Linux CAN layer.
The esd_pci331 driver from the SVN repository is for an intelligent
card, if I remember correctly. But using an intelligent card just for
the raw CAN protocol seems kind of overkill to me.
> We should probably start a new thread about this, it is getting
> off-topic for Pavel's CAN driver. Is the socketcan mailing list ok for
> this kind of discussion?
Of course, if it's about Socket-CAN.
Wolfgang.
> Thanks,
> Ira
> _______________________________________________
> Socketcan-core mailing list
> Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> https://lists.berlios.de/mailman/listinfo/socketcan-core
>
>
^ permalink raw reply
* Re: [BUG]? xfrm: INFO: inconsistent lock state
From: Herbert Xu @ 2010-02-02 7:49 UTC (permalink / raw)
To: Wei Yongjun; +Cc: netdev@vger.kernel.org, David S. Miller, Yury Polyanskiy
In-Reply-To: <4B66A670.70503@cn.fujitsu.com>
On Mon, Feb 01, 2010 at 06:01:20PM +0800, Wei Yongjun wrote:
> I got following lock INFO in my Fedora12 box uner 2.6.33-rc6.
> Is this a BUG?
Apparently.
This is introduced by
commit 9e0d57fd6dad37d72a3ca6db00ca8c76f2215454
Author: Yury Polyanskiy <polyanskiy@gmail.com>
Date: Sun Nov 8 20:58:41 2009 -0800
xfrm: SAD entries do not expire correctly after suspend-resume
To be honest I never liked that patch since the semantic change
makes sense for suspend-resume but is totally the wrong thing to
do when the system clock is stepped/changed.
Whether this is a real dead-lock depends on whether hrtimers
behave as real hard IRQs. If they do then yes it can deadlock.
So my suggestion would be to just revert this change and consider
an alternative solution to the original suspend-resume problem.
My preference would be something that is triggered through power-
management as that would then allows us to differentiate between
the suspend-resume case and the clock change case.
It could even be triggered from user-space as it knows when an
resume occurs.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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
* [PATCH 05/10] drivers/net/davinci_emac.c: Fix continuation line formats
From: Joe Perches @ 2010-02-02 7:22 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev
In-Reply-To: <cover.1265095093.git.joe@perches.com>
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/davinci_emac.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 33c4fe2..faffad4 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -2672,8 +2672,7 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
priv->emac_base_phys = res->start + pdata->ctrl_reg_offset;
size = res->end - res->start + 1;
if (!request_mem_region(res->start, size, ndev->name)) {
- dev_err(emac_dev, "DaVinci EMAC: failed request_mem_region() \
- for regs\n");
+ dev_err(emac_dev, "DaVinci EMAC: failed request_mem_region() for regs\n");
rc = -ENXIO;
goto probe_quit;
}
--
1.6.6.rc0.57.gad7a
^ permalink raw reply related
* [PATCH 00/10] Non-printk continuation line fixes
From: Joe Perches @ 2010-02-02 7:22 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, Mark Brown, linux-ia64, kvm, David Airlie,
alsa-devel, Jaroslav Kysela, Grant Likely, linux-ide, devel,
David Brownell, linux-scsi, lm-sensors, Takashi Iwai, Scott Fang,
Avi Kivity, dri-devel, Jeff Garzik, Xiantao Zhang, Andrew Vasquez,
Fenghua Yu, Roger Lucas, Greg Kroah-Hartman, James E.J. Bottomley,
Liam Girdwood, Tony
Some more fixes to formats with continuation lines
Joe Perches (10):
arch/ia64: Fix continuation line formats
drivers/ata: Fix continuation line formats
drivers/gpu/drm/i915/intel_bios.c: Fix continuation line formats
drivers/hwmon/vt8231.c: Fix continuation line formats
drivers/net/davinci_emac.c: Fix continuation line formats
drivers/scsi/qla2xxx/qua_os.c: Fix continuation line formats
drivers/spi/spi_s3c64xx.c: Fix continuation line formats
drivers/staging: Fix continuation line formats
drivers/video/via: Fix continuation line formats
sound/soc/s3c24xx/s2c-pcm.c: Fix continuation line formats
arch/ia64/kvm/mmio.c | 4 ++--
arch/ia64/kvm/vcpu.c | 4 ++--
drivers/ata/pata_at91.c | 4 ++--
drivers/gpu/drm/i915/intel_bios.c | 3 +--
drivers/hwmon/vt8231.c | 3 +--
drivers/net/davinci_emac.c | 3 +--
drivers/scsi/qla2xxx/qla_os.c | 4 ++--
drivers/spi/spi_s3c64xx.c | 11 +++++------
drivers/staging/rtl8192su/r8192S_phy.c | 16 ++++++++--------
drivers/staging/sep/sep_driver.c | 5 +++--
drivers/staging/slicoss/slicoss.c | 6 ++----
drivers/video/via/dvi.c | 8 ++++----
drivers/video/via/lcd.c | 12 ++++++------
sound/soc/s3c24xx/s3c-pcm.c | 3 +--
14 files changed, 40 insertions(+), 46 deletions(-)
------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
--
^ permalink raw reply
* Re: [PATCH] netfilter: per netns nf_conntrack_cachep
From: Jon Masters @ 2010-02-02 7:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexey Dobriyan, Patrick McHardy, linux-kernel, netdev,
netfilter-devel, Paul E. McKenney
In-Reply-To: <1265085391.2861.106.camel@tonnant>
On Mon, 2010-02-01 at 23:36 -0500, Jon Masters wrote:
> On Mon, 2010-02-01 at 15:52 +0100, Eric Dumazet wrote:
>
> > [PATCH] netfilter: per netns nf_conntrack_cachep
> >
> > nf_conntrack_cachep is currently shared by all netns instances, but
> > because of SLAB_DESTROY_BY_RCU special semantics, this is wrong.
> >
> > If we use a shared slab cache, one object can instantly flight between
> > one hash table (netns ONE) to another one (netns TWO), and concurrent
> > reader (doing a lookup in netns ONE, 'finding' an object of netns TWO)
> > can be fooled without notice, because no RCU grace period has to be
> > observed between object freeing and its reuse.
> >
> > We dont have this problem with UDP/TCP slab caches because TCP/UDP
> > hashtables are global to the machine (and each object has a pointer to
> > its netns).
> >
> > If we use per netns conntrack hash tables, we also *must* use per netns
> > conntrack slab caches, to guarantee an object can not escape from one
> > namespace to another one.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> You're totally right, I'd missed this (RCU behavior wrt SLAB caches was
> one of these black magic voodoo things until Peter Z. set me straight
> with his explanation that it only applies to the freeing of the cache
> itself, not the objects - that makes sense in the grand scheme of what
> RCU is trying to achieve, and so in theory, yeah we could just verify
> the ct object we get back out of the cache is from the same ns, should
> work just as well as doing per-ns caches, but not as clean IMO). I'm
> still not sure it explains the specific corruption I'm seeing, but I
> just made some coffee and put on some T. Rex to help me think.
It happens in the kmem_cache_free in nf_conntrack_free (as I can
trivially confirm with printk), so you're almost certainly right, but I
am just shoving in a bunch of debug code equivalent to the logic in SLUB
kmem cache freeing to see precisely how that pointer corruption occurs,
mostly for curiosity, and to confirm for sure what happens.
Jon.
^ permalink raw reply
* [PATCH] sunrpc/cache: fix module refcnt leak in a failure path
From: Li Zefan @ 2010-02-02 6:48 UTC (permalink / raw)
To: NeilBrown, J. Bruce Fields; +Cc: David Miller, LKML, netdev@vger.kernel.org
Don't forget to release the module refcnt if seq_open()
returns failure.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
net/sunrpc/cache.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 39bddba..b7af6b2 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1233,8 +1233,10 @@ static int content_open(struct inode *inode, struct file *file,
if (!cd || !try_module_get(cd->owner))
return -EACCES;
han = __seq_open_private(file, &cache_content_op, sizeof(*han));
- if (han == NULL)
+ if (han == NULL) {
+ module_put(cd->owner);
return -ENOMEM;
+ }
han->cd = cd;
return 0;
--
1.6.3
^ permalink raw reply related
* RE: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode
From: Liu Yu-B13201 @ 2010-02-02 5:57 UTC (permalink / raw)
To: Grant Likely; +Cc: galak, davem, netdev, linuxppc-dev
In-Reply-To: <fa686aa41002011357pb24d8a6k640ab5ae13f8fbbc@mail.gmail.com>
Thanks.
As the patchset is accepted.
Will commit another one to fix them.
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On
> Behalf Of Grant Likely
> Sent: Tuesday, February 02, 2010 5:58 AM
> To: Liu Yu-B13201
> Cc: galak@kernel.crashing.org; davem@davemloft.net;
> netdev@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode
>
> On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > ---
> > arch/powerpc/platforms/85xx/mpc85xx_mds.c | 24
> ++++++++++++++++++++++++
> > 1 files changed, 24 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > index c5028a2..0872e4a 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > @@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void)
> > } else if (machine_is(mpc8569_mds)) {
> > #define BCSR7_UCC12_GETHnRST (0x1 << 2)
> > #define BCSR8_UEM_MARVELL_RST (0x1 << 1)
> > +#define BCSR_UCC_RGMII (0x1 << 6)
> > +#define BCSR_UCC_RTBI (0x1 << 5)
> > /*
> > * U-Boot mangles interrupt polarity
> for Marvell PHYs,
> > * so reset built-in and UEM Marvell
> PHYs, this puts
> > @@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void)
> >
> > setbits8(&bcsr_regs[7],
> BCSR7_UCC12_GETHnRST);
> > clrbits8(&bcsr_regs[8],
> BCSR8_UEM_MARVELL_RST);
> > +
> > + for (np = NULL; (np =
> of_find_compatible_node(np,
> > + "network",
>
> Don't match on the 'type' field. Replace "network" with NULL and just
> rely on "ucc_geth" for matching.
>
> > +
> "ucc_geth")) != NULL;) {
> > + const unsigned int *prop;
>
> u32 please.
>
> Also, rather than reusing 'prop' for both char* and u32 values, which
> forces you to use ugly casts, use 2 local variables here.
>
> > + int ucc_num;
> > +
> > + prop = of_get_property(np,
> "cell-index", NULL);
> > + if (prop == NULL)
> > + continue;
> > +
> > + ucc_num = *prop - 1;
>
> Ugh. No bounds checking...
>
> > +
> > + prop = of_get_property(np,
> "phy-connection-type", NULL);
> > + if (prop == NULL)
> > + continue;
> > +
> > + if (strcmp("rtbi", (const
> char *)prop) == 0)
>
> (This is the ugly cast I was talking about.)
>
> > +
> clrsetbits_8(&bcsr_regs[7 + ucc_num],
>
> ...not having bounds checking could result in badness in this
> array index.
>
> This patch is dangerous as written.
>
> Finally, while using cell-index seems convenient, I think it would be
> better to have a lookup table of the index into the BCSR register
> block from the UCC base address, which also gives you implicit bounds
> checking.
>
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>
>
^ permalink raw reply
* Re: [PATCHSET] percpu: add __percpu sparse annotations
From: Tejun Heo @ 2010-02-02 5:37 UTC (permalink / raw)
To: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
Trond.Myklebust, aelder, hch@
In-Reply-To: <1264432935-10453-1-git-send-email-tj@kernel.org>
On 01/26/2010 12:22 AM, Tejun Heo wrote:
> If nobody objects, I'll push these into percpu tree in three or four
> days.
Patchset pushed out w/ the array workarounds dropped.
Thanks.
--
tejun
^ permalink raw reply
* Re: [net-next-2.6 PATCH 2/5] if_link: Add SR-IOV configuration methods
From: Or Gerlitz @ 2010-02-02 5:16 UTC (permalink / raw)
To: Williams, Mitch A
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, Or Gerlitz
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FDD55C136@rrsmsx501.amr.corp.intel.com>
Williams, Mitch A <mitch.a.williams@intel.com> wrote:
> To be honest, Or, this completely slipped my mind. I'll add it to my to-do list.
> If it's OK with you, I'd prefer to get this set of patches in as-is and
> add the egress qos stuff later. A lot of people have been asking for
> these patches, so I'd rather not delay for this feature.
Mitch, looking on the patch series I see that except for two patches
([PATCH 1/5] pci: Add SR-IOV convenience functions and macros and
[PATCH 2/4] libnetlink: Modify parser to track the first duplicated
attributes) seven other patches have to be changed/enhanced to
incorporate the egress qos mapping. If you prefer to do that later
rather then now I am fine with that, but lets not leave that a side.
Or.
^ permalink raw reply
* Re: [PATCH] netfilter: per netns nf_conntrack_cachep
From: Jon Masters @ 2010-02-02 4:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexey Dobriyan, Patrick McHardy, linux-kernel, netdev,
netfilter-devel, Paul E. McKenney
In-Reply-To: <1265035970.2848.50.camel@edumazet-laptop>
On Mon, 2010-02-01 at 15:52 +0100, Eric Dumazet wrote:
> [PATCH] netfilter: per netns nf_conntrack_cachep
>
> nf_conntrack_cachep is currently shared by all netns instances, but
> because of SLAB_DESTROY_BY_RCU special semantics, this is wrong.
>
> If we use a shared slab cache, one object can instantly flight between
> one hash table (netns ONE) to another one (netns TWO), and concurrent
> reader (doing a lookup in netns ONE, 'finding' an object of netns TWO)
> can be fooled without notice, because no RCU grace period has to be
> observed between object freeing and its reuse.
>
> We dont have this problem with UDP/TCP slab caches because TCP/UDP
> hashtables are global to the machine (and each object has a pointer to
> its netns).
>
> If we use per netns conntrack hash tables, we also *must* use per netns
> conntrack slab caches, to guarantee an object can not escape from one
> namespace to another one.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
You're totally right, I'd missed this (RCU behavior wrt SLAB caches was
one of these black magic voodoo things until Peter Z. set me straight
with his explanation that it only applies to the freeing of the cache
itself, not the objects - that makes sense in the grand scheme of what
RCU is trying to achieve, and so in theory, yeah we could just verify
the ct object we get back out of the cache is from the same ns, should
work just as well as doing per-ns caches, but not as clean IMO). I'm
still not sure it explains the specific corruption I'm seeing, but I
just made some coffee and put on some T. Rex to help me think.
Jon.
P.S. What's up with all the "Welcome, Mr. Bond" and "i_see_dead_people"
and other comments in that code anyway? If you're going to use movie
references, perhaps standardize on one particular genre :)
^ permalink raw reply
* [PATCH net-next-2.6] bonding: Remove net_device_stats from bonding struct
From: Ajit Khaparde @ 2010-02-02 0:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Ajit Khaparde
There is no need to maintain stats in the bonding structure.
Use the instance of net_device_stats in netdevice.
Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
drivers/net/bonding/bond_main.c | 2 +-
drivers/net/bonding/bonding.h | 1 -
2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6221936..1787e3c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3742,7 +3742,7 @@ static int bond_close(struct net_device *bond_dev)
static struct net_device_stats *bond_get_stats(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct net_device_stats *stats = &bond->stats;
+ struct net_device_stats *stats = &bond_dev->stats;
struct net_device_stats local_stats;
struct slave *slave;
int i;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 558ec13..257a7a4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -197,7 +197,6 @@ struct bonding {
s8 send_grat_arp;
s8 send_unsol_na;
s8 setup_by_slave;
- struct net_device_stats stats;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
char proc_file_name[IFNAMSIZ];
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/2] sky2: fix transmit DMA map leakage
From: Stephen Hemminger @ 2010-02-01 23:41 UTC (permalink / raw)
To: David Miller; +Cc: Jarek Poplawski, Michael Breuer, netdev
The book keeping structure for transmit always had the flags value
cleared so transmit DMA maps were never released correctly.
Based on patch by Jarek Poplawski, problem observed by Michael Breuer.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--
This patch is against net-next but it should go to 2.6.33 and stable
as well.
--- a/drivers/net/sky2.c 2010-02-01 10:07:42.676296236 -0800
+++ b/drivers/net/sky2.c 2010-02-01 10:18:12.575044064 -0800
@@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sk
static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
{
struct sky2_tx_le *le = sky2->tx_le + *slot;
- struct tx_ring_info *re = sky2->tx_ring + *slot;
*slot = RING_NEXT(*slot, sky2->tx_ring_size);
- re->flags = 0;
- re->skb = NULL;
le->ctrl = 0;
return le;
}
@@ -1622,8 +1619,7 @@ static unsigned tx_le_req(const struct s
return count;
}
-static void sky2_tx_unmap(struct pci_dev *pdev,
- const struct tx_ring_info *re)
+static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
{
if (re->flags & TX_MAP_SINGLE)
pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
@@ -1633,6 +1629,7 @@ static void sky2_tx_unmap(struct pci_dev
pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
pci_unmap_len(re, maplen),
PCI_DMA_TODEVICE);
+ re->flags = 0;
}
/*
@@ -1839,6 +1836,7 @@ static void sky2_tx_complete(struct sky2
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
+ re->skb = NULL;
dev_kfree_skb_any(skb);
sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
^ permalink raw reply
* [PATCH 2/2] sky2: hand receive DMA mapping failures
From: Stephen Hemminger @ 2010-02-01 23:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, Jarek Poplawski, Michael Breuer, netdev
In-Reply-To: <20100201154147.5bb0b2c7@nehalam>
If receive buffer mapping failed, then it was possible to get
stuck with unmapped receive buffer in DMA ring.
This would be an extremely rare condition because the driver had just
released the map for the last receive so it should be able to get
another map again (in soft-irq).
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
Patch is against net-next. Not worth the risk for 2.6.33 or stable
--- a/drivers/net/sky2.c 2010-02-01 10:18:12.575044064 -0800
+++ b/drivers/net/sky2.c 2010-02-01 10:18:16.975356378 -0800
@@ -1103,18 +1103,39 @@ static int sky2_rx_map_skb(struct pci_de
int i;
re->data_addr = pci_map_single(pdev, skb->data, size, PCI_DMA_FROMDEVICE);
- if (unlikely(pci_dma_mapping_error(pdev, re->data_addr)))
- return -EIO;
+ if (pci_dma_mapping_error(pdev, re->data_addr))
+ goto mapping_error;
pci_unmap_len_set(re, data_size, size);
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
- re->frag_addr[i] = pci_map_page(pdev,
- skb_shinfo(skb)->frags[i].page,
- skb_shinfo(skb)->frags[i].page_offset,
- skb_shinfo(skb)->frags[i].size,
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+ re->frag_addr[i] = pci_map_page(pdev, frag->page,
+ frag->page_offset,
+ frag->size,
PCI_DMA_FROMDEVICE);
+
+ if (pci_dma_mapping_error(pdev, re->frag_addr[i]))
+ goto map_page_error;
+ }
return 0;
+
+map_page_error:
+ while (--i >= 0) {
+ pci_unmap_page(pdev, re->frag_addr[i],
+ skb_shinfo(skb)->frags[i].size,
+ PCI_DMA_FROMDEVICE);
+ }
+
+ pci_unmap_single(pdev, re->data_addr, pci_unmap_len(re, data_size),
+ PCI_DMA_FROMDEVICE);
+
+mapping_error:
+ if (net_ratelimit())
+ dev_warn(&pdev->dev, "%s: rx mapping error\n",
+ skb->dev->name);
+ return -EIO;
}
static void sky2_rx_unmap_skb(struct pci_dev *pdev, struct rx_ring_info *re)
@@ -2306,30 +2327,32 @@ static struct sk_buff *receive_new(struc
struct rx_ring_info *re,
unsigned int length)
{
- struct sk_buff *skb, *nskb;
+ struct sk_buff *skb;
+ struct rx_ring_info nre;
unsigned hdr_space = sky2->rx_data_size;
- /* Don't be tricky about reusing pages (yet) */
- nskb = sky2_rx_alloc(sky2);
- if (unlikely(!nskb))
- return NULL;
+ nre.skb = sky2_rx_alloc(sky2);
+ if (unlikely(!nre.skb))
+ goto nobuf;
+
+ if (sky2_rx_map_skb(sky2->hw->pdev, &nre, hdr_space))
+ goto nomap;
skb = re->skb;
sky2_rx_unmap_skb(sky2->hw->pdev, re);
-
prefetch(skb->data);
- re->skb = nskb;
- if (sky2_rx_map_skb(sky2->hw->pdev, re, hdr_space)) {
- dev_kfree_skb(nskb);
- re->skb = skb;
- return NULL;
- }
+ *re = nre;
if (skb_shinfo(skb)->nr_frags)
skb_put_frags(skb, hdr_space, length);
else
skb_put(skb, length);
return skb;
+
+nomap:
+ dev_kfree_skb(nre.skb);
+nobuf:
+ return NULL;
}
/*
^ permalink raw reply
* Re: [PATCH v3] sky2: receive dma mapping error handling
From: Stephen Hemminger @ 2010-02-01 22:51 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201224639.GA4664@del.dom.local>
On Mon, 1 Feb 2010 23:46:39 +0100
Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Mon, Feb 01, 2010 at 02:29:42PM -0800, Stephen Hemminger wrote:
> > On Mon, 1 Feb 2010 22:27:41 +0100
> > Jarek Poplawski <jarkao2@gmail.com> wrote:
> >
> > > @@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
> > > {
> > > struct sky2_tx_le *le;
> > >
> > > - sky2->tx_prod = sky2->tx_cons = 0;
> > > + sky2->tx_prod = 0;
> > > sky2->tx_tcpsum = 0;
> > > sky2->tx_last_mss = 0;
> > >
> > > le = get_tx_le(sky2, &sky2->tx_prod);
> > > le->addr = 0;
> > > le->opcode = OP_ADDR64 | HW_OWNER;
> > > + sky2->tx_cons = sky2->tx_prod;
> > > sky2->tx_last_upper = 0;
> > > }
> >
> > Your change causes the initial element to be skipped. I want
> > it to goto the hardware. It makes sure the upper bits of the
> > first request are set (0).
>
> I thought "Send high bits if needed" part in sky2_xmit_frame() was
> enough. If it's otherwise than my patch was wrong.
>
The definition is "high bits are different than the last value".
The code init sets the last value to zero, and initializes the hardware
engine as well.
^ permalink raw reply
* Re: [PATCH v3] sky2: receive dma mapping error handling
From: Jarek Poplawski @ 2010-02-01 22:46 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201142942.710273cf@nehalam>
On Mon, Feb 01, 2010 at 02:29:42PM -0800, Stephen Hemminger wrote:
> On Mon, 1 Feb 2010 22:27:41 +0100
> Jarek Poplawski <jarkao2@gmail.com> wrote:
>
> > @@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
> > {
> > struct sky2_tx_le *le;
> >
> > - sky2->tx_prod = sky2->tx_cons = 0;
> > + sky2->tx_prod = 0;
> > sky2->tx_tcpsum = 0;
> > sky2->tx_last_mss = 0;
> >
> > le = get_tx_le(sky2, &sky2->tx_prod);
> > le->addr = 0;
> > le->opcode = OP_ADDR64 | HW_OWNER;
> > + sky2->tx_cons = sky2->tx_prod;
> > sky2->tx_last_upper = 0;
> > }
>
> Your change causes the initial element to be skipped. I want
> it to goto the hardware. It makes sure the upper bits of the
> first request are set (0).
I thought "Send high bits if needed" part in sky2_xmit_frame() was
enough. If it's otherwise than my patch was wrong.
>
> I don't see what was wrong with my fix.
>
It's OK. Please, submit it instead of mine.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH v3] sky2: receive dma mapping error handling
From: Stephen Hemminger @ 2010-02-01 22:29 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201212740.GA3289@del.dom.local>
On Mon, 1 Feb 2010 22:27:41 +0100
Jarek Poplawski <jarkao2@gmail.com> wrote:
> @@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
> {
> struct sky2_tx_le *le;
>
> - sky2->tx_prod = sky2->tx_cons = 0;
> + sky2->tx_prod = 0;
> sky2->tx_tcpsum = 0;
> sky2->tx_last_mss = 0;
>
> le = get_tx_le(sky2, &sky2->tx_prod);
> le->addr = 0;
> le->opcode = OP_ADDR64 | HW_OWNER;
> + sky2->tx_cons = sky2->tx_prod;
> sky2->tx_last_upper = 0;
> }
Your change causes the initial element to be skipped. I want
it to goto the hardware. It makes sure the upper bits of the
first request are set (0).
I don't see what was wrong with my fix.
--
^ permalink raw reply
* Re: [PATCH 3/4] ucc_geth: update the tbi-phy setting
From: Grant Likely @ 2010-02-01 22:00 UTC (permalink / raw)
To: Liu Yu; +Cc: galak, davem, netdev, linuxppc-dev
In-Reply-To: <1263456799-3306-3-git-send-email-yu.liu@freescale.com>
On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> Old method only set tbi-phy for eth0.
>
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> drivers/net/ucc_geth.c | 23 +++++++++++++++--------
> 1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 750d86d..0e72013 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -37,6 +37,7 @@
> #include <asm/qe.h>
> #include <asm/ucc.h>
> #include <asm/ucc_fast.h>
> +#include <asm/machdep.h>
>
> #include "ucc_geth.h"
> #include "fsl_pq_mdio.h"
> @@ -1334,7 +1335,7 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
> struct ucc_geth __iomem *ug_regs;
> struct ucc_fast __iomem *uf_regs;
> int ret_val;
> - u32 upsmr, maccfg2, tbiBaseAddress;
> + u32 upsmr, maccfg2;
> u16 value;
>
> ugeth_vdbg("%s: IN", __func__);
> @@ -1389,14 +1390,20 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
> /* Note that this depends on proper setting in utbipar register. */
> if ((ugeth->phy_interface == PHY_INTERFACE_MODE_TBI) ||
> (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
> - tbiBaseAddress = in_be32(&ug_regs->utbipar);
> - tbiBaseAddress &= UTBIPAR_PHY_ADDRESS_MASK;
> - tbiBaseAddress >>= UTBIPAR_PHY_ADDRESS_SHIFT;
> - value = ugeth->phydev->bus->read(ugeth->phydev->bus,
> - (u8) tbiBaseAddress, ENET_TBI_MII_CR);
> + struct ucc_geth_info *ug_info = ugeth->ug_info;
> + struct phy_device *tbiphy;
> +
> + if (!ug_info->tbi_node)
> + ugeth_warn("TBI mode requires that the device "
> + "tree specify a tbi-handle\n");
Shouldn't the driver avoid calling of_phy_find_device() if this error
path is taken?
> +
> + tbiphy = of_phy_find_device(ug_info->tbi_node);
> + if (!tbiphy)
> + ugeth_warn("Could not get TBI device\n");
> +
> + value = phy_read(tbiphy, ENET_TBI_MII_CR);
Ditto here; the code unconditionally tries to read the tbiphy, even if
it isn't there.
Looks wrong.
g.
> value &= ~0x1000; /* Turn off autonegotiation */
> - ugeth->phydev->bus->write(ugeth->phydev->bus,
> - (u8) tbiBaseAddress, ENET_TBI_MII_CR, value);
> + phy_write(tbiphy, ENET_TBI_MII_CR, value);
> }
>
> init_check_frame_length_mode(ug_info->lengthCheckRx, &ug_regs->maccfg2);
> --
> 1.6.4
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode
From: Grant Likely @ 2010-02-01 21:57 UTC (permalink / raw)
To: Liu Yu; +Cc: galak, davem, netdev, linuxppc-dev
In-Reply-To: <1263456799-3306-2-git-send-email-yu.liu@freescale.com>
On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> arch/powerpc/platforms/85xx/mpc85xx_mds.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index c5028a2..0872e4a 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void)
> } else if (machine_is(mpc8569_mds)) {
> #define BCSR7_UCC12_GETHnRST (0x1 << 2)
> #define BCSR8_UEM_MARVELL_RST (0x1 << 1)
> +#define BCSR_UCC_RGMII (0x1 << 6)
> +#define BCSR_UCC_RTBI (0x1 << 5)
> /*
> * U-Boot mangles interrupt polarity for Marvell PHYs,
> * so reset built-in and UEM Marvell PHYs, this puts
> @@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void)
>
> setbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
> clrbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
> +
> + for (np = NULL; (np = of_find_compatible_node(np,
> + "network",
Don't match on the 'type' field. Replace "network" with NULL and just
rely on "ucc_geth" for matching.
> + "ucc_geth")) != NULL;) {
> + const unsigned int *prop;
u32 please.
Also, rather than reusing 'prop' for both char* and u32 values, which
forces you to use ugly casts, use 2 local variables here.
> + int ucc_num;
> +
> + prop = of_get_property(np, "cell-index", NULL);
> + if (prop == NULL)
> + continue;
> +
> + ucc_num = *prop - 1;
Ugh. No bounds checking...
> +
> + prop = of_get_property(np, "phy-connection-type", NULL);
> + if (prop == NULL)
> + continue;
> +
> + if (strcmp("rtbi", (const char *)prop) == 0)
(This is the ugly cast I was talking about.)
> + clrsetbits_8(&bcsr_regs[7 + ucc_num],
...not having bounds checking could result in badness in this array index.
This patch is dangerous as written.
Finally, while using cell-index seems convenient, I think it would be
better to have a lookup table of the index into the BCSR register
block from the UCC base address, which also gives you implicit bounds
checking.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/4] dts/mpc8569mds: Cleanup tbi phy to support rtbi
From: Grant Likely @ 2010-02-01 21:46 UTC (permalink / raw)
To: Liu Yu; +Cc: netdev, linuxppc-dev, davem
In-Reply-To: <1263456799-3306-1-git-send-email-yu.liu@freescale.com>
On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> After these change, when need to work in rtbi mode,
> just change phy-connection-type to "rtbi".
>
> Also, this work can be done by u-boot.
>
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
Looks okay by me,
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> arch/powerpc/boot/dts/mpc8569mds.dts | 47 ++++++++++++++++++++++++++++++---
> 1 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
> index 795eb36..8b72eaf 100644
> --- a/arch/powerpc/boot/dts/mpc8569mds.dts
> +++ b/arch/powerpc/boot/dts/mpc8569mds.dts
> @@ -535,6 +535,7 @@
> rx-clock-name = "none";
> tx-clock-name = "clk12";
> pio-handle = <&pio1>;
> + tbi-handle = <&tbi1>;
> phy-handle = <&qe_phy0>;
> phy-connection-type = "rgmii-id";
> };
> @@ -579,7 +580,7 @@
> reg = <0x6>;
> device_type = "ethernet-phy";
> };
> - tbi-phy@11 {
> + tbi1: tbi-phy@11 {
> reg = <0x11>;
> device_type = "tbi-phy";
> };
> @@ -590,7 +591,7 @@
> reg = <0x3520 0x18>;
> compatible = "fsl,ucc-mdio";
>
> - tbi0: tbi-phy@15 {
> + tbi6: tbi-phy@15 {
> reg = <0x15>;
> device_type = "tbi-phy";
> };
> @@ -600,7 +601,7 @@
> #size-cells = <0>;
> reg = <0x3720 0x38>;
> compatible = "fsl,ucc-mdio";
> - tbi1: tbi-phy@17 {
> + tbi8: tbi-phy@17 {
> reg = <0x17>;
> device_type = "tbi-phy";
> };
> @@ -617,10 +618,22 @@
> rx-clock-name = "none";
> tx-clock-name = "clk12";
> pio-handle = <&pio3>;
> + tbi-handle = <&tbi3>;
> phy-handle = <&qe_phy2>;
> phy-connection-type = "rgmii-id";
> };
>
> + mdio@2320 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x2320 0x18>;
> + compatible = "fsl,ucc-mdio";
> + tbi3: tbi-phy@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> +
> enet1: ucc@3000 {
> device_type = "network";
> compatible = "ucc_geth";
> @@ -632,10 +645,22 @@
> rx-clock-name = "none";
> tx-clock-name = "clk17";
> pio-handle = <&pio2>;
> + tbi-handle = <&tbi2>;
> phy-handle = <&qe_phy1>;
> phy-connection-type = "rgmii-id";
> };
>
> + mdio@3120 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x3120 0x18>;
> + compatible = "fsl,ucc-mdio";
> + tbi2: tbi-phy@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> +
> enet3: ucc@3200 {
> device_type = "network";
> compatible = "ucc_geth";
> @@ -647,10 +672,22 @@
> rx-clock-name = "none";
> tx-clock-name = "clk17";
> pio-handle = <&pio4>;
> + tbi-handle = <&tbi4>;
> phy-handle = <&qe_phy3>;
> phy-connection-type = "rgmii-id";
> };
>
> + mdio@3320 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x3320 0x18>;
> + compatible = "fsl,ucc-mdio";
> + tbi4: tbi-phy@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> +
> enet5: ucc@3400 {
> device_type = "network";
> compatible = "ucc_geth";
> @@ -661,7 +698,7 @@
> local-mac-address = [ 00 00 00 00 00 00 ];
> rx-clock-name = "none";
> tx-clock-name = "none";
> - tbi-handle = <&tbi0>;
> + tbi-handle = <&tbi6>;
> phy-handle = <&qe_phy5>;
> phy-connection-type = "sgmii";
> };
> @@ -676,7 +713,7 @@
> local-mac-address = [ 00 00 00 00 00 00 ];
> rx-clock-name = "none";
> tx-clock-name = "none";
> - tbi-handle = <&tbi1>;
> + tbi-handle = <&tbi8>;
> phy-handle = <&qe_phy7>;
> phy-connection-type = "sgmii";
> };
> --
> 1.6.4
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH v3b resent] sky2: Fix transmit dma mapping handling
From: Jarek Poplawski @ 2010-02-01 21:42 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201102018.7b597992@nehalam>
[ Subject was wrong, sorry! ]
On Mon, Feb 01, 2010 at 10:20:18AM -0800, Stephen Hemminger wrote:
> This fixes the fact that re->flags is always zero without causing
> other confusion.
Here it is, with only sky2->tx_cons update in tx_init().
Thanks,
Jarek P.
---------------> (take 3b)
Michael Breuer reported that dma-debug entries added by sky2 driver
weren't unmapped, and found out "re->flags is always NULL upon entry
to sky2_tx_unmap". It is overwritten by get_tx_le() after changes
introduced by commit 6b84dacadbdc3dab6a5b313d20d5a93b0d998641.
This patch moves re->flags and re->skb initializations from
get_tx_le() to sky2_tx_unmap() and sky2_tx_complete() respectively.
Additionally sky2->tx_cons is updated to sky2->tx_prod in tx_init()
to remove one needless tx completion.
With debugging by: Michael Breuer <mbreuer@majjas.com>
Improved by: Stephen Hemminger <shemminger@vyatta.com>
Reported-by: Michael Breuer <mbreuer@majjas.com>
Tested-by: Michael Breuer <mbreuer@majjas.com>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index d760650..08cd65b 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sky2_hw *hw, u32 qaddr,
static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
{
struct sky2_tx_le *le = sky2->tx_le + *slot;
- struct tx_ring_info *re = sky2->tx_ring + *slot;
*slot = RING_NEXT(*slot, sky2->tx_ring_size);
- re->flags = 0;
- re->skb = NULL;
le->ctrl = 0;
return le;
}
@@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
{
struct sky2_tx_le *le;
- sky2->tx_prod = sky2->tx_cons = 0;
+ sky2->tx_prod = 0;
sky2->tx_tcpsum = 0;
sky2->tx_last_mss = 0;
le = get_tx_le(sky2, &sky2->tx_prod);
le->addr = 0;
le->opcode = OP_ADDR64 | HW_OWNER;
+ sky2->tx_cons = sky2->tx_prod;
sky2->tx_last_upper = 0;
}
@@ -1622,8 +1620,7 @@ static unsigned tx_le_req(const struct sk_buff *skb)
return count;
}
-static void sky2_tx_unmap(struct pci_dev *pdev,
- const struct tx_ring_info *re)
+static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
{
if (re->flags & TX_MAP_SINGLE)
pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
@@ -1633,6 +1630,7 @@ static void sky2_tx_unmap(struct pci_dev *pdev,
pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
pci_unmap_len(re, maplen),
PCI_DMA_TODEVICE);
+ re->flags = 0;
}
/*
@@ -1839,6 +1837,7 @@ static void sky2_tx_complete(struct sky2_port *sky2, u16 done)
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
+ re->skb = NULL;
dev_kfree_skb_any(skb);
sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
^ permalink raw reply related
* [PATCH v3] sky2: receive dma mapping error handling
From: Jarek Poplawski @ 2010-02-01 21:27 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201102018.7b597992@nehalam>
On Mon, Feb 01, 2010 at 10:20:18AM -0800, Stephen Hemminger wrote:
> This fixes the fact that re->flags is always zero without causing
> other confusion.
Here it is, with only sky2->tx_cons update in tx_init().
Thanks,
Jarek P.
---------------> (take 3)
Michael Breuer reported that dma-debug entries added by sky2 driver
weren't unmapped, and found out "re->flags is always NULL upon entry
to sky2_tx_unmap". It is overwritten by get_tx_le() after changes
introduced by commit 6b84dacadbdc3dab6a5b313d20d5a93b0d998641.
This patch moves re->flags and re->skb initializations from
get_tx_le() to sky2_tx_unmap() and sky2_tx_complete() respectively.
Additionally sky2->tx_cons is updated to sky2->tx_prod in tx_init()
to remove one needless tx completion.
With debugging by: Michael Breuer <mbreuer@majjas.com>
Improved by: Stephen Hemminger <shemminger@vyatta.com>
Reported-by: Michael Breuer <mbreuer@majjas.com>
Tested-by: Michael Breuer <mbreuer@majjas.com>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index d760650..08cd65b 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sky2_hw *hw, u32 qaddr,
static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
{
struct sky2_tx_le *le = sky2->tx_le + *slot;
- struct tx_ring_info *re = sky2->tx_ring + *slot;
*slot = RING_NEXT(*slot, sky2->tx_ring_size);
- re->flags = 0;
- re->skb = NULL;
le->ctrl = 0;
return le;
}
@@ -1038,13 +1035,14 @@ static void tx_init(struct sky2_port *sky2)
{
struct sky2_tx_le *le;
- sky2->tx_prod = sky2->tx_cons = 0;
+ sky2->tx_prod = 0;
sky2->tx_tcpsum = 0;
sky2->tx_last_mss = 0;
le = get_tx_le(sky2, &sky2->tx_prod);
le->addr = 0;
le->opcode = OP_ADDR64 | HW_OWNER;
+ sky2->tx_cons = sky2->tx_prod;
sky2->tx_last_upper = 0;
}
@@ -1622,8 +1620,7 @@ static unsigned tx_le_req(const struct sk_buff *skb)
return count;
}
-static void sky2_tx_unmap(struct pci_dev *pdev,
- const struct tx_ring_info *re)
+static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
{
if (re->flags & TX_MAP_SINGLE)
pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
@@ -1633,6 +1630,7 @@ static void sky2_tx_unmap(struct pci_dev *pdev,
pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
pci_unmap_len(re, maplen),
PCI_DMA_TODEVICE);
+ re->flags = 0;
}
/*
@@ -1839,6 +1837,7 @@ static void sky2_tx_complete(struct sky2_port *sky2, u16 done)
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
+ re->skb = NULL;
dev_kfree_skb_any(skb);
sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);
^ permalink raw reply related
* Re: [PATCH] irda: remove BKL from irnet open function
From: Arnd Bergmann @ 2010-02-01 21:20 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: John Kacur, Samuel Ortiz, David S. Miller, linux-kernel, netdev
In-Reply-To: <20100201203616.GJ1414@holoscopio.com>
On Monday 01 February 2010, Thadeu Lima de Souza Cascardo wrote:
> On Mon, Feb 01, 2010 at 09:32:30PM +0100, John Kacur wrote:
> > On Mon, Feb 1, 2010 at 7:18 PM, Thadeu Lima de Souza Cascardo
> > <cascardo@holoscopio.com> wrote:
> And is it possible that ioctl will be called before open returns? If it
> is, then, yes, this is not safe. But I don't really believe the case. Or
> is it?
ioctl may be called on an open file descriptor while open is called
by another thread to open a second file descriptor for the same device.
> Or is it only possible to happen with different struct file*? In that
> case, open is only allocating and initializing the irnet_socket *ap.
> Then, ioctl uses it. There is some race between the different ioctls,
> but no race between open/ioctl for different opened devices. That is, a
> process may open /dev/irnet while another process is issuing ioctls to
> its own opened /dev/irnet.
right.
> Besides, dev_irnet_ioctl uses the file private_data to get to the
> irnet_socket, which is the last thing the open call does. I assume doing
> an attribution to a pointer is atomic in all architectures supported by
> Linux currently, isn't it?
The pointer assignment is atomic, but it may not be synchronized to data
pointed to it. On Alpha (probably no others so far), this would result
in irnet_socket seen as uninitialized after the pointer to is can be
seen as valid if there was no locking. No architecture would read an
invalid pointer though.
I guess that what John was trying to point out actually is the fact that
you shouldn't really do the BKL removal in one function only but rather
do it for the whole driver at once. The irnet driver uses the BKL in
open, ioctl and llseek, so if you want to clean up that driver, please
introduce proper locking in the driver and do all of the three.
Arnd
^ permalink raw reply
* Re: [net-next-2.6 v2] can: add support for CAN interface cards based on the PLX90xx PCI bridge
From: Ira W. Snyder @ 2010-02-01 21:05 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4B673A0A.5080504-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Mon, Feb 01, 2010 at 09:31:06PM +0100, Wolfgang Grandegger wrote:
> Ira W. Snyder wrote:
> > On Tue, Feb 02, 2010 at 01:42:44AM +0600, Pavel B. Cheblakov wrote:
> >> From: Pavel Cheblakov <P.B.Cheblakov-tHBF8D5G73F4OK5fxMSSsQ@public.gmane.org>
> >>
> >> This driver is for CAN interface cards based on the PLX90xx PCI bridge.
> >> Driver supports now:
> >> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/)
> >> - Adlink PCI-7841/cPCI-7841 SE card
> >> - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
> >> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
> >>
> >> Changes since v1:
> >> - Added some defines for static inline int plx_pci_check_sja1000(...)
> >> - static struct pci_device_id plx_pci_tbl[] replaced by
> >> static DEFINE_PCI_DEVICE_TABLE(plx_pci_tbl)
> >> - Typo fixed
> >>
> >
> > Hello,
> >
> > Any ideas if this driver could support the Janz PCI CAN cards as well? I
> > don't have any documentation for the cards, and I'd love to get away
> > from their closed-source drivers.
>
> Is it a passive or and intelligent CAN PCI card? What is the name of the
> card? A intelligent card will run its own firmware and you do not have
> direct access to the SJA1000. Pavel plx_pci driver is quite generic and
> it should not be a big deal to support a PLX90xx based passive card.
>
It is an intelligent CAN PCI card. I'm pretty sure they're the card
described here: http://www.janz.de/as/en/can-cp6/k2.html
Since this driver is for passive cards, it clearly won't work. Are there
any examples of drivers written for intelligent CAN cards? Until I found
the website above, I'd never seen any data sheets for this card. They
look like they may have enough information to write a driver to
interface with the new Linux CAN layer.
We should probably start a new thread about this, it is getting
off-topic for Pavel's CAN driver. Is the socketcan mailing list ok for
this kind of discussion?
Thanks,
Ira
^ permalink raw reply
* Re: [net-next-2.6 v2] can: add support for CAN interface cards based on the PLX90xx PCI bridge
From: Wolfgang Grandegger @ 2010-02-01 20:48 UTC (permalink / raw)
To: Pavel B. Cheblakov
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1265053364-24099-1-git-send-email-chebl-LmX6Lu7C9G7nhuLbLO4Grw@public.gmane.org>
Pavel B. Cheblakov wrote:
> From: Pavel Cheblakov <P.B.Cheblakov-tHBF8D5G73F4OK5fxMSSsQ@public.gmane.org>
>
> This driver is for CAN interface cards based on the PLX90xx PCI bridge.
> Driver supports now:
> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/)
> - Adlink PCI-7841/cPCI-7841 SE card
> - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
>
> Changes since v1:
> - Added some defines for static inline int plx_pci_check_sja1000(...)
> - static struct pci_device_id plx_pci_tbl[] replaced by
> static DEFINE_PCI_DEVICE_TABLE(plx_pci_tbl)
> - Typo fixed
>
> Signed-off-by: Pavel Cheblakov <P.B.Cheblakov-tHBF8D5G73F4OK5fxMSSsQ@public.gmane.org>
> Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Looks good now. The interface seems now generic enough to support other
PLX09xx based CAN cards as well.
Thanks for your contribution.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] sky2: receive dma mapping error handling
From: Jarek Poplawski @ 2010-02-01 20:41 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael Breuer, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100201201323.GA3096@del.dom.local>
On Mon, Feb 01, 2010 at 09:13:23PM +0100, Jarek Poplawski wrote:
> On Mon, Feb 01, 2010 at 10:20:18AM -0800, Stephen Hemminger wrote:
> > This fixes the fact that re->flags is always zero without causing
> > other confusion.
>
> Actually, there is a slight confusion: after tx_init() slots #0 are
> skipped and waiting for tx_complete. Of course, no big deal, but no
> problem with fixing it too, so there is the main difference between
Hmm... On the other hand it could be fixed simpler by moving
sky2->tx_cons. I'll send v3.
Thanks,
Jarek P.
^ 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