* [PATCH 2/2] agp/uninorth: Unify U3 and pre-U3 insert_memory and remove_memory hooks.
From: Michel Dänzer @ 2009-12-06 12:15 UTC (permalink / raw)
To: David Airlie; +Cc: linuxppc-dev
In-Reply-To: <1260101756-22645-1-git-send-email-michel@daenzer.net>
From: Michel Dänzer <daenzer@vmware.com>
Signed-off-by: Michel Dänzer <daenzer@vmware.com>
---
drivers/char/agp/uninorth-agp.c | 64 +++++++--------------------------------
1 files changed, 11 insertions(+), 53 deletions(-)
diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 4e05021..d89da4a 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -144,53 +144,7 @@ static int uninorth_configure(void)
return 0;
}
-static int uninorth_insert_memory(struct agp_memory *mem, off_t pg_start,
- int type)
-{
- int i, j, num_entries;
- void *temp;
- int mask_type;
-
- if (type != mem->type)
- return -EINVAL;
-
- mask_type = agp_bridge->driver->agp_type_to_mask_type(agp_bridge, type);
- if (mask_type != 0) {
- /* We know nothing of memory types */
- return -EINVAL;
- }
-
- if (mem->page_count == 0)
- return 0;
-
- temp = agp_bridge->current_size;
- num_entries = A_SIZE_32(temp)->num_entries;
-
- if ((pg_start + mem->page_count) > num_entries)
- return -EINVAL;
-
- j = pg_start;
-
- while (j < (pg_start + mem->page_count)) {
- if (agp_bridge->gatt_table[j])
- return -EBUSY;
- j++;
- }
-
- for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
- agp_bridge->gatt_table[j] =
- cpu_to_le32((page_to_phys(mem->pages[i]) & 0xFFFFF000UL) | 0x1UL);
- flush_dcache_range((unsigned long)__va(page_to_phys(mem->pages[i])),
- (unsigned long)__va(page_to_phys(mem->pages[i]))+0x1000);
- }
- (void)in_le32((volatile u32*)&agp_bridge->gatt_table[pg_start]);
- mb();
-
- uninorth_tlbflush(mem);
- return 0;
-}
-
-static int u3_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
+static int uninorth_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
{
int i, num_entries;
void *temp;
@@ -219,14 +173,18 @@ static int u3_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
for (i = 0; i < mem->page_count; ++i) {
if (gp[i]) {
dev_info(&agp_bridge->dev->dev,
- "u3_insert_memory: entry 0x%x occupied (%x)\n",
+ "uninorth_insert_memory: entry 0x%x occupied (%x)\n",
i, gp[i]);
return -EBUSY;
}
}
for (i = 0; i < mem->page_count; i++) {
- gp[i] = (page_to_phys(mem->pages[i]) >> PAGE_SHIFT) | 0x80000000UL;
+ if (is_u3)
+ gp[i] = (page_to_phys(mem->pages[i]) >> PAGE_SHIFT) | 0x80000000UL;
+ else
+ gp[i] = cpu_to_le32((page_to_phys(mem->pages[i]) & 0xFFFFF000UL) |
+ 0x1UL);
flush_dcache_range((unsigned long)__va(page_to_phys(mem->pages[i])),
(unsigned long)__va(page_to_phys(mem->pages[i]))+0x1000);
}
@@ -236,7 +194,7 @@ static int u3_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
return 0;
}
-int u3_remove_memory(struct agp_memory *mem, off_t pg_start, int type)
+int uninorth_remove_memory(struct agp_memory *mem, off_t pg_start, int type)
{
size_t i;
u32 *gp;
@@ -551,7 +509,7 @@ const struct agp_bridge_driver uninorth_agp_driver = {
.create_gatt_table = uninorth_create_gatt_table,
.free_gatt_table = uninorth_free_gatt_table,
.insert_memory = uninorth_insert_memory,
- .remove_memory = agp_generic_remove_memory,
+ .remove_memory = uninorth_remove_memory,
.alloc_by_type = agp_generic_alloc_by_type,
.free_by_type = agp_generic_free_by_type,
.agp_alloc_page = agp_generic_alloc_page,
@@ -577,8 +535,8 @@ const struct agp_bridge_driver u3_agp_driver = {
.agp_enable = uninorth_agp_enable,
.create_gatt_table = uninorth_create_gatt_table,
.free_gatt_table = uninorth_free_gatt_table,
- .insert_memory = u3_insert_memory,
- .remove_memory = u3_remove_memory,
+ .insert_memory = uninorth_insert_memory,
+ .remove_memory = uninorth_remove_memory,
.alloc_by_type = agp_generic_alloc_by_type,
.free_by_type = agp_generic_free_by_type,
.agp_alloc_page = agp_generic_alloc_page,
--
1.6.5.3
^ permalink raw reply related
* [PATCH 1/2] agp/uninorth: Also handle user memory types in u3_remove_memory().
From: Michel Dänzer @ 2009-12-06 12:15 UTC (permalink / raw)
To: David Airlie; +Cc: linuxppc-dev
In-Reply-To: <1260101756-22645-1-git-send-email-michel@daenzer.net>
From: Michel Dänzer <daenzer@vmware.com>
Also short-circuit empty updates.
Signed-off-by: Michel Dänzer <daenzer@vmware.com>
---
drivers/char/agp/uninorth-agp.c | 29 ++++++++++++++++++++++-------
1 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 703959e..4e05021 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -151,9 +151,6 @@ static int uninorth_insert_memory(struct agp_memory *mem, off_t pg_start,
void *temp;
int mask_type;
- temp = agp_bridge->current_size;
- num_entries = A_SIZE_32(temp)->num_entries;
-
if (type != mem->type)
return -EINVAL;
@@ -163,6 +160,12 @@ static int uninorth_insert_memory(struct agp_memory *mem, off_t pg_start,
return -EINVAL;
}
+ if (mem->page_count == 0)
+ return 0;
+
+ temp = agp_bridge->current_size;
+ num_entries = A_SIZE_32(temp)->num_entries;
+
if ((pg_start + mem->page_count) > num_entries)
return -EINVAL;
@@ -194,9 +197,6 @@ static int u3_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
u32 *gp;
int mask_type;
- temp = agp_bridge->current_size;
- num_entries = A_SIZE_32(temp)->num_entries;
-
if (type != mem->type)
return -EINVAL;
@@ -206,6 +206,12 @@ static int u3_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
return -EINVAL;
}
+ if (mem->page_count == 0)
+ return 0;
+
+ temp = agp_bridge->current_size;
+ num_entries = A_SIZE_32(temp)->num_entries;
+
if ((pg_start + mem->page_count) > num_entries)
return -EINVAL;
@@ -234,10 +240,19 @@ int u3_remove_memory(struct agp_memory *mem, off_t pg_start, int type)
{
size_t i;
u32 *gp;
+ int mask_type;
+
+ if (type != mem->type)
+ return -EINVAL;
- if (type != 0 || mem->type != 0)
+ mask_type = agp_bridge->driver->agp_type_to_mask_type(agp_bridge, type);
+ if (mask_type != 0) {
/* We know nothing of memory types */
return -EINVAL;
+ }
+
+ if (mem->page_count == 0)
+ return 0;
gp = (u32 *) &agp_bridge->gatt_table[pg_start];
for (i = 0; i < mem->page_count; ++i)
--
1.6.5.3
^ permalink raw reply related
* [RESEND][PATCH 0/2] Uninorth AGP fix / cleanup
From: Michel Dänzer @ 2009-12-06 12:15 UTC (permalink / raw)
To: David Airlie; +Cc: linuxppc-dev
Looks like these got lost somewhere last time, so trying again...
These patches should allow KMS environments to work with U3 AGP bridges as
well.
[PATCH 1/2] agp/uninorth: Also handle user memory types in u3_remove_memory().
[PATCH 2/2] agp/uninorth: Unify U3 and pre-U3 insert_memory and remove_memory hooks.
^ permalink raw reply
* [RFC] doc/powerpc: try to explain why the interrupt numbers are off by 16
From: Sebastian Andrzej Siewior @ 2009-12-06 11:32 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Sebastian Andrzej Siewior, Alemao, linux-kernel
In-Reply-To: <4AA97D50.40209@freescale.com>
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
Documentation/powerpc/dts-bindings/fsl/mpic.txt | 42 +++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/fsl/mpic.txt
diff --git a/Documentation/powerpc/dts-bindings/fsl/mpic.txt b/Documentation/powerpc/dts-bindings/fsl/mpic.txt
new file mode 100644
index 0000000..71e39cf
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/fsl/mpic.txt
@@ -0,0 +1,42 @@
+* OpenPIC and its interrupt numbers on Freescale's e500/e600 cores
+
+The OpenPIC specification does not specify which interrupt source has to
+become which interrupt number. This is up to the software implementation
+of the interrupt controller. The only requirement is that every
+interrupt source has to have an unique interrupt number / vector number.
+To accomplish this the current implementation assigns the number zero to
+the first source, the number one to the second source and so on until
+all interrupt sources have their unique number.
+Usually the assigned vector number equals the interrupt number mentioned
+in the documentation for a given core / CPU. This is however not true
+for the e500 cores (MPC85XX CPUs) where the documentation distinguishes
+between internal and external interrupt sources and starts counting at
+zero for both of them.
+
+So what to write for external interrupt source X or internal interrupt
+source Y into the device tree? Here is an example:
+
+The memory map for the interrupt controller in the MPC8544[0] shows,
+that the first interrupt source starts at 0x5_0000 (PIC Register Address
+Map-Interrupt Source Configuration Registers). This source becomes the
+number zero therefore:
+ External interrupt 0 = interrupt number 0
+ External interrupt 1 = interrupt number 1
+ External interrupt 2 = interrupt number 2
+ ...
+Every interrupt number allocates 0x20 bytes register space. So to get
+its number it is sufficient to shift the lower 16bits to right by five.
+So for the external interrupt 10 we have:
+ 0x0140 >> 5 = 10
+
+After the external sources, the internal sources follow. The in core I2C
+controller on the MPC8544 for instance has the internal source number
+27. Oo obtain its interrupt number we take the lower 16bits of its memory
+address (0x5_0560) and shift it right:
+ 0x0560 >> 5 = 43
+
+Therefore the I2C device node for the MPC8544 CPU has to have the
+interrupt number 43 specified in the device tree.
+
+[0] MPC8544E PowerQUICCTM III, Integrated Host Processor Family Reference Manual
+ MPC8544ERM Rev. 1 10/2007
--
1.6.5.3
^ permalink raw reply related
* Re: TQM5200 + SM501 FB
From: Wolfgang Denk @ 2009-12-06 9:17 UTC (permalink / raw)
To: Rolf Offermanns; +Cc: linuxppc-dev
In-Reply-To: <F5B2E7AE-A0C5-4D9B-AF8F-3158B94D6236@sysgo.com>
Dear Rolf Offermanns,
In message <F5B2E7AE-A0C5-4D9B-AF8F-3158B94D6236@sysgo.com> you wrote:
>
> Thanks Wolfgang, I will try it on monday. What needs to be done to get
> this into mainline?
>
> Modify the sm501 stuff for fdt and extend the tqm5200.dts file with the
> sm501 resources?
Right, especially the OF bindings in the SM501 driver and the device
tree entries are still missing.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
He'd been wrong, there _was_ a light at the end of the tunnel, and it
was a flamethrower. - Terry Pratchett, _Mort_
^ permalink raw reply
* Re: TQM5200 + SM501 FB
From: Rolf Offermanns @ 2009-12-06 8:30 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev
In-Reply-To: <20091204220426.ADD063F6E3@gemini.denx.de>
Am 04.12.2009 um 23:04 schrieb Wolfgang Denk:
> Please find attached below a patch which fixes some issues (but we
> don't consider it clean enough for inclusion into mainline yet), and a
> known to be working config file. [Kudos to Anatolij for providing
> this.]
>=20
Thanks Wolfgang, I will try it on monday. What needs to be done to get =
this into mainline?
Modify the sm501 stuff for fdt and extend the tqm5200.dts file with the =
sm501 resources?
-Rolf=
^ permalink raw reply
* ZILOG serial port broken in 2.6.32
From: Rob Landley @ 2009-12-06 7:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus
Trying again with a few likely-looking cc's from the MAINTAINERS file:
Summary:
The PMACZILOG serial driver last worked in 2.6.28. It was broken by commit
f751928e0ddf54ea4fe5546f35e99efc5b5d9938 by Alan Cox making bits of the tty
layer dynamically allocated. The PMACZILOG driver wasn't properly converted,
it works with interrupts disabled (for boot messages), but as soon as
interrupts are enabled (PID 1 spawns) the next write to the serial console
panics the kernel.
Up through 2.6.31 I could fix it by reverting that patch (which isn't a proper
fix but it made it work). In 2.6.32 the patch no longer cleanly reverts.
I reported the issue here (with a cut and paste of the panic trace):
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-October/076727.html
And reported the results of bisecting the issue here:
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-October/077059.html
I noted that 2.6.32-pre had broken my workaround here:
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-December/078498.html
Background:
I have a project that builds the same native Linux development environment for
multiple hardware targets. It aims to support all the targets QEMU system
emulation can boot Linux under, although I'm still a few short. It creates a
cross compiler and uses it to build a root filesystem from uClibc and busybox,
adds a native toolchain, and packages it up into a system image (squashfs,
ext2, or initramfs depending on the config you selected).
Anyone can then boot the resulting system image under qemu and use it to wet
source and compile stuff natively. (If the cross compiler is in the $PATH on
the host, it will even configure distcc to call out to that cross compiler to
speed up the builds to merely "painfully slow", with some pretense of SMP
scalability).
Prebuilt binaries of all the targets I had working last release are at
http://impactlinux.com/fwl/downloads/binaries (with obligatory screenshots at
http://impactlinux.com/fwl/screenshots/ even). They use the 2.6.31 kernel.
It supports powerpc. If you look at system-image-powerpc.tar.bz2 you'll see
that the run-emulator.sh script has been using qemu's "g3beige" target board
emulation, which provides all the hardware I need for a development
environment (hard drive, network card, at least 256 megs of memory, working
clock chip, and of course a serial console). Userspace doesn't care what I
use, it's the same processor instruction set and same C library either way,
the board emulation's just something to boot it on, only the kernel .config
really cares as long as the appropriate resources are there.
Unfortunately, g3beige seems to have bit-rotted, and thus the serial console
is now panicing. This is a regression, and thus blocking a release of my
project using the 2.6.32 kernel. Is this of interest to anyone other than me?
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
^ permalink raw reply
* 4th attempt: ZILOG serial console broken in 2.6.32
From: Rob Landley @ 2009-12-06 6:29 UTC (permalink / raw)
To: linuxppc-dev
My posts are showing up in the archive...
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-October/076727.html
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-October/077059.html
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-December/078498.html
But no replies. Is there somebody specific I should be addressing this to?
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
^ permalink raw reply
* Re: [v10 PATCH 2/9]: cpuidle: cleanup drivers/cpuidle/cpuidle.c
From: Arun R Bharadwaj @ 2009-12-06 5:19 UTC (permalink / raw)
To: Torsten Duwe
Cc: linux-arch, Peter Zijlstra, Venkatesh Pallipadi, linux-kernel,
linux-acpi, Arun Bharadwaj, Ingo Molnar, linuxppc-dev
In-Reply-To: <200912042320.01320.duwe@lst.de>
* Torsten Duwe <duwe@lst.de> [2009-12-04 23:20:00]:
> On Wednesday 02 December 2009, Arun R Bharadwaj wrote:
> > * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-12-02 15:24:27]:
> >
> > This patch cleans up drivers/cpuidle/cpuidle.c
> > Earlier cpuidle assumed pm_idle as the default idle loop. Break that
> > assumption and make it more generic.
>
> Is there a problem with the old pm_idle? Couldn't it be integrated more
> transparently, instead of replacing it this intrusively?
>
Hi Torsten,
Peter objected to the idea of integrating this with the old pm_idle
because it has already caused a lot of problems on x86 and we wouldn't
want to be doing the same mistake on POWER. The discussion related to
that could be found here http://lkml.org/lkml/2009/8/26/233
> > --- linux.trees.git.orig/include/linux/cpuidle.h
> > +++ linux.trees.git/include/linux/cpuidle.h
> > @@ -41,7 +41,7 @@ struct cpuidle_state {
> > unsigned long long usage;
> > unsigned long long time; /* in US */
> >
> > - int (*enter) (struct cpuidle_device *dev,
> > + void (*enter) (struct cpuidle_device *dev,
> > struct cpuidle_state *state);
> > };
>
> While it may be a good idea to move the residency calculation to one central
> place, at least in theory a cpuidle_state->enter() function could have a
> better method to determine its value.
>
This would mean a lot of code replication, which Pavel pointed out in
the previous iteration. So I moved the residency calculation to a
central place.
> Either way you're implicitly introducing an API change here, and you're at
> least missing two functions on ARM and SuperH, respectively. Could you
> separate this API change out, and not take it for granted in the other
> patches?
>
> Torsten
^ permalink raw reply
* Re: using different format for hugetlbfs
From: Kumar Gala @ 2009-12-06 3:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linux-ppc list, David Gibson
In-Reply-To: <1259961942.2076.1277.camel@pasglop>
On Dec 4, 2009, at 3:25 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2009-12-04 at 08:09 -0600, Kumar Gala wrote:
>> On Dec 4, 2009, at 2:58 AM, Benjamin Herrenschmidt wrote:
>>
>>> On Fri, 2009-12-04 at 01:18 -0600, Kumar Gala wrote:
>>>> Ben, David,
>>>>
>>>> If we want to support true 4G/4G split on ppc32 using the MSB of
>>>> the
>>>> address to determine of the pgd_t is for hugetlbfs isn't going to
>>>> work. Since every pointer in the pgd_t -> pud_t -> pmd_t is
>>>> point to
>>>> at least a 4K page I would think the low order 12-bits should
>>>> always
>>>> be 0.
>>>
>>> On 32 bit maybe. On 64, the pg/u/md's can be smaller. I don't really
>>> want to have a different encoding for both types though.
>>
>> What do you mean they can be smaller? We have some scenario when we
>> dont allocate a full page? I agree having the encodings be different
>> would be bad. I'm trying to avoid having it be different between 32
>> bit and 64 (but maybe that will be impossible).
>
> Yes. The intermediary levels are smaller on 64-bit. Also, with
> hugetlbfs
> it can create special levels of various sizes depending on the
> requirements to fit a given huge page size. And that would be true of
> both 32 and 64-bit in fact.
Even than, does that preclude the format I suggested? I'm assuming
that pgd_t/pud_t/pmd_t are always a double word so the low order 4-
bits should be 0 (on 64-bit), so using the lsb as the flag between
hugetlb and normal pointer should still work.
- k
^ permalink raw reply
* Re: [PATCH] powerpc: mpc8xxx_gpio: Add ability to mask off unused GPIO pins
From: Anton Vorontsov @ 2009-12-05 20:51 UTC (permalink / raw)
To: Peter Tyser; +Cc: linuxppc-dev
In-Reply-To: <1260041552.8643.33.camel@ptyser-laptop>
On Sat, Dec 05, 2009 at 01:32:32PM -0600, Peter Tyser wrote:
[...]
> > > Adding a new "fsl,gpio-mask" device tree property allows a dts file to
> > > accurately describe what GPIO pins are available for use on a given
> > > board.
> >
> > I don't see any real usage for this. If device tree specifies a wrong
> > gpio in the gpios = <> property, then it's a bug in the device tree
> > and should be fixed (or workarounded in the platform code).
> >
> > If a user fiddles with unknown gpios via sysfs interface, then it's
> > user's problem.
>
> Its the sysfs case that I'm concerned about. Primarily because:
> 1. Users scratch their head when they see that the "ngpio" sysfs value
> doesn't match their CPU manual or board vendor's manual, and
> subsequently ask their board vendor's engineers (ie me:) what's up.
I don't think that adding code and device tree entries just for
documentation purposes is a good idea.
> 2. Improperly using GPIO pins could damage hardware for some boards.
Well, your initial patch tried to solve a different problem: to not
let users to request non-existent GPIOs, which is usually safe.
[...]
> #2 could be worked around by exporting GPIO pins in platform code so
> that they are not available via sysfs.
Yes, badly designed hardware deserves ugly hacks in the platform
code. ;-) So for this problem, just request these gpios in the
platform code.
> Would it be any more acceptable to instead add
> a "fsl,num-gpio" property so that "ngpio" actually reported an accurate
> value and non-existent GPIO pins couldn't be used/exported?
I'd think it's actually less acceptable. fsl,gpio-mask is more generic,
since from gpio-mask you can deduce ngpio. But it's still ugly.
What would be OK to do is to describe in the device tree every
device that is using some GPIO, and then let the userspace request
*only* gpios that are described in the device-tree. That way you
can automatically exclude not-existent gpios.
And if some gpios are just headers on the board, you can still
describe them in the device tree via "gpio-header" nodes.
Still, a lot of efforts for no real gain...
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH] Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
From: Benjamin Herrenschmidt @ 2009-12-05 20:25 UTC (permalink / raw)
To: Grant Likely; +Cc: Asier Llano, netdev, David S. Miller, linuxppc-dev
In-Reply-To: <20091204211737.26998.58504.stgit@angua>
On Fri, 2009-12-04 at 14:20 -0700, Grant Likely wrote:
> From: Asier Llano Palacios <asierllano@gmail.com>
>
> net/mpc5200: Fix locking on fec_mpc52xx driver
>
> Fix the locking scheme on the fec_mpc52xx driver. This device can
> receive IRQs from three sources; the FEC itself, the tx DMA, and the
> rx DMA. Mutual exclusion was handled by taking a spin_lock() in the
> critical regions, but because the handlers are run with IRQs enabled,
> spin_lock() is insufficient and the driver can end up interrupting
> a critical region anyway from another IRQ.
.../...
I would suggest a simpler locking strategy.
The network stack is going to provide you with a lock already which is
the tx queue lock. It's going to be taken for you in start_xmit.
That gives you a simple locking against tx using existing network stack
lock unlock primitives
Everything else should happen within NAPI poll.
The trick then is to not need locking in your interrupt handlers. You
shouldn't need locking to schedule NAPI which is all they should do for
normal tx/rx interrupts. For errors, just kick a workqueue.
>From that work queue, you can stop NAPI and stop the TX while you do
your chip reset etc...
Something like what tg3 does:
static inline void tg3_netif_stop(struct tg3 *tp)
{
tp->dev->trans_start = jiffies; /* prevent tx timeout */
tg3_napi_disable(tp);
netif_tx_disable(tp->dev);
}
You may want to be careful about potential calls to some of your other
network callbacks tho, such as set_multicast.. I don't think the
network stack will shield you there and set_multicast, last I looked,
was called in a context where you can't use a mutex. But that gives you
the basic locking strategy for your driver without adding an unnecessary
spinlock to your fast path.
Cheers,
Ben.
> Asier Llano discovered that this occurs when an error IRQ is raised
> in the middle of handling rx irqs which resulted in an sk_buff memory
> leak.
>
> In addition, locking is spotty at best in the driver and inspection
> revealed quite a few places with insufficient locking.
>
> This patch is based on Asier's initial work, but reworks a number of
> things so that locks are held for as short a time as possible, so
> that spin_lock_irqsave() is used everywhere, and so the locks are
> dropped when calling into the network stack (because the lock only
> protects the hardware interface; not the network stack).
>
> Boot tested on a lite5200 with an NFS root. Has not been performance
> tested.
>
> Signed-off-by: Asier Llano <a.llano@ziv.es>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> Asier, can you please test this? It took me a while to respond to
> your initial post because I was concerned about some of the latency
> issues, and I was concerned about disabling IRQs for long periods in
> the RX handler. I think it should be good now, but it needs testing.
>
> Cheers,
> g.
>
> drivers/net/fec_mpc52xx.c | 121 +++++++++++++++++++++++----------------------
> 1 files changed, 62 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index 66dace6..4889b4d 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -85,11 +85,15 @@ MODULE_PARM_DESC(debug, "debugging messages level");
>
> static void mpc52xx_fec_tx_timeout(struct net_device *dev)
> {
> + struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> + unsigned long flags;
> +
> dev_warn(&dev->dev, "transmit timed out\n");
>
> + spin_lock_irqsave(&priv->lock, flags);
> mpc52xx_fec_reset(dev);
> -
> dev->stats.tx_errors++;
> + spin_unlock_irqrestore(&priv->lock, flags);
>
> netif_wake_queue(dev);
> }
> @@ -135,28 +139,32 @@ static void mpc52xx_fec_free_rx_buffers(struct net_device *dev, struct bcom_task
> }
> }
>
> +static void
> +mpc52xx_fec_rx_submit(struct net_device *dev, struct sk_buff *rskb)
> +{
> + struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> + struct bcom_fec_bd *bd;
> +
> + bd = (struct bcom_fec_bd *) bcom_prepare_next_buffer(priv->rx_dmatsk);
> + bd->status = FEC_RX_BUFFER_SIZE;
> + bd->skb_pa = dma_map_single(dev->dev.parent, rskb->data,
> + FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> + bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
> +}
> +
> static int mpc52xx_fec_alloc_rx_buffers(struct net_device *dev, struct bcom_task *rxtsk)
> {
> - while (!bcom_queue_full(rxtsk)) {
> - struct sk_buff *skb;
> - struct bcom_fec_bd *bd;
> + struct sk_buff *skb;
>
> + while (!bcom_queue_full(rxtsk)) {
> skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
> - if (skb == NULL)
> + if (!skb)
> return -EAGAIN;
>
> /* zero out the initial receive buffers to aid debugging */
> memset(skb->data, 0, FEC_RX_BUFFER_SIZE);
> -
> - bd = (struct bcom_fec_bd *)bcom_prepare_next_buffer(rxtsk);
> -
> - bd->status = FEC_RX_BUFFER_SIZE;
> - bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
> - FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> -
> - bcom_submit_next_buffer(rxtsk, skb);
> + mpc52xx_fec_rx_submit(dev, skb);
> }
> -
> return 0;
> }
>
> @@ -328,13 +336,12 @@ static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
> DMA_TO_DEVICE);
>
> bcom_submit_next_buffer(priv->tx_dmatsk, skb);
> + spin_unlock_irqrestore(&priv->lock, flags);
>
> if (bcom_queue_full(priv->tx_dmatsk)) {
> netif_stop_queue(dev);
> }
>
> - spin_unlock_irqrestore(&priv->lock, flags);
> -
> return NETDEV_TX_OK;
> }
>
> @@ -359,9 +366,9 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
> {
> struct net_device *dev = dev_id;
> struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> + unsigned long flags;
>
> - spin_lock(&priv->lock);
> -
> + spin_lock_irqsave(&priv->lock, flags);
> while (bcom_buffer_done(priv->tx_dmatsk)) {
> struct sk_buff *skb;
> struct bcom_fec_bd *bd;
> @@ -372,11 +379,10 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
>
> dev_kfree_skb_irq(skb);
> }
> + spin_unlock_irqrestore(&priv->lock, flags);
>
> netif_wake_queue(dev);
>
> - spin_unlock(&priv->lock);
> -
> return IRQ_HANDLED;
> }
>
> @@ -384,67 +390,60 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
> {
> struct net_device *dev = dev_id;
> struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> + struct sk_buff *rskb; /* received sk_buff */
> + struct sk_buff *skb; /* new sk_buff to enqueue in its place */
> + struct bcom_fec_bd *bd;
> + u32 status, physaddr;
> + int length;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->lock, flags);
>
> while (bcom_buffer_done(priv->rx_dmatsk)) {
> - struct sk_buff *skb;
> - struct sk_buff *rskb;
> - struct bcom_fec_bd *bd;
> - u32 status;
>
> rskb = bcom_retrieve_buffer(priv->rx_dmatsk, &status,
> - (struct bcom_bd **)&bd);
> - dma_unmap_single(dev->dev.parent, bd->skb_pa, rskb->len,
> - DMA_FROM_DEVICE);
> + (struct bcom_bd **)&bd);
> + physaddr = bd->skb_pa;
>
> /* Test for errors in received frame */
> if (status & BCOM_FEC_RX_BD_ERRORS) {
> /* Drop packet and reuse the buffer */
> - bd = (struct bcom_fec_bd *)
> - bcom_prepare_next_buffer(priv->rx_dmatsk);
> -
> - bd->status = FEC_RX_BUFFER_SIZE;
> - bd->skb_pa = dma_map_single(dev->dev.parent,
> - rskb->data,
> - FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> -
> - bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
> -
> + mpc52xx_fec_rx_submit(dev, rskb);
> dev->stats.rx_dropped++;
> -
> continue;
> }
>
> /* skbs are allocated on open, so now we allocate a new one,
> * and remove the old (with the packet) */
> skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
> - if (skb) {
> - /* Process the received skb */
> - int length = status & BCOM_FEC_RX_BD_LEN_MASK;
> -
> - skb_put(rskb, length - 4); /* length without CRC32 */
> -
> - rskb->dev = dev;
> - rskb->protocol = eth_type_trans(rskb, dev);
> -
> - netif_rx(rskb);
> - } else {
> + if (!skb) {
> /* Can't get a new one : reuse the same & drop pkt */
> - dev_notice(&dev->dev, "Memory squeeze, dropping packet.\n");
> + dev_notice(&dev->dev, "Low memory - dropped packet.\n");
> + mpc52xx_fec_rx_submit(dev, rskb);
> dev->stats.rx_dropped++;
> -
> - skb = rskb;
> + continue;
> }
>
> - bd = (struct bcom_fec_bd *)
> - bcom_prepare_next_buffer(priv->rx_dmatsk);
> + /* Enqueue the new sk_buff back on the hardware */
> + mpc52xx_fec_rx_submit(dev, skb);
>
> - bd->status = FEC_RX_BUFFER_SIZE;
> - bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
> - FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> + /* Process the received skb - Drop the spin lock while
> + * calling into the network stack */
> + spin_unlock_irqrestore(&priv->lock, flags);
>
> - bcom_submit_next_buffer(priv->rx_dmatsk, skb);
> + dma_unmap_single(dev->dev.parent, physaddr, rskb->len,
> + DMA_FROM_DEVICE);
> + length = status & BCOM_FEC_RX_BD_LEN_MASK;
> + skb_put(rskb, length - 4); /* length without CRC32 */
> + rskb->dev = dev;
> + rskb->protocol = eth_type_trans(rskb, dev);
> + netif_rx(rskb);
> +
> + spin_lock_irqsave(&priv->lock, flags);
> }
>
> + spin_unlock_irqrestore(&priv->lock, flags);
> +
> return IRQ_HANDLED;
> }
>
> @@ -454,6 +453,7 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
> struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> struct mpc52xx_fec __iomem *fec = priv->fec;
> u32 ievent;
> + unsigned long flags;
>
> ievent = in_be32(&fec->ievent);
>
> @@ -471,9 +471,10 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
> if (net_ratelimit() && (ievent & FEC_IEVENT_XFIFO_ERROR))
> dev_warn(&dev->dev, "FEC_IEVENT_XFIFO_ERROR\n");
>
> + spin_lock_irqsave(&priv->lock, flags);
> mpc52xx_fec_reset(dev);
> + spin_unlock_irqrestore(&priv->lock, flags);
>
> - netif_wake_queue(dev);
> return IRQ_HANDLED;
> }
>
> @@ -768,6 +769,8 @@ static void mpc52xx_fec_reset(struct net_device *dev)
> bcom_enable(priv->tx_dmatsk);
>
> mpc52xx_fec_start(dev);
> +
> + netif_wake_queue(dev);
> }
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH] powerpc: mpc8xxx_gpio: Add ability to mask off unused GPIO pins
From: Peter Tyser @ 2009-12-05 19:32 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <20091205175614.GA10057@oksana.dev.rtsoft.ru>
Hi Anton,
Thanks for the feedback.
> On Fri, Dec 04, 2009 at 01:43:40PM -0600, Peter Tyser wrote:
> > This change resolves 2 issues:
> > - Different chips have a different number of GPIO pins per controller.
> > For example, the MPC8347 has 32, the P2020 16, and the mpc8572 8.
> > Previously, the mpc8xxx_gpio driver assumed every chip had 32 GPIO
> > pins which resulted in some processors reporting an incorrect 'ngpio'
> > field in /sys. Additionally, users could export and "use" 32 GPIO
> > pins, although in reality only a subset of the 32 pins had any real
> > functionality.
> >
> > - Some boards don't utilize all available GPIO pins. Previously,
> > unused GPIO pins could still be exported and "used", even though the
> > pins had no real functionality. This is somewhat confusing to a user
> > and also allow a user to do something "bad", like change an unused
> > floating output into a floating input.
>
> There are hundreds of other ways to screw things up.
>
> Think of /dev/mem, you still able to change the registers.
> Before changing any GPIO (whether it is a normal or reserved GPIO),
> user has to consult with schematics/docs.
Agreed. This is an attempt to make it just a little bit harder to
accidentally screw things up and to make the "ngpio" sysfs value
actually contain an accurate value.
> > Adding a new "fsl,gpio-mask" device tree property allows a dts file to
> > accurately describe what GPIO pins are available for use on a given
> > board.
>
> I don't see any real usage for this. If device tree specifies a wrong
> gpio in the gpios = <> property, then it's a bug in the device tree
> and should be fixed (or workarounded in the platform code).
>
> If a user fiddles with unknown gpios via sysfs interface, then it's
> user's problem.
Its the sysfs case that I'm concerned about. Primarily because:
1. Users scratch their head when they see that the "ngpio" sysfs value
doesn't match their CPU manual or board vendor's manual, and
subsequently ask their board vendor's engineers (ie me:) what's up.
2. Improperly using GPIO pins could damage hardware for some boards.
For example, some of our boards have a voltage regulator controlled via
GPIO pins so that a CPU's core voltage can be changed based on its
frequency, etc. A user could damage their CPU if they aren't careful
with those GPIO pins. For pins like that, it'd be nice to not even let
users play with them.
#2 could be worked around by exporting GPIO pins in platform code so
that they are not available via sysfs. And I agree that if a user is
playing with GPIO pins, they had better know what they are doing, so #1
above is my main issue. Would it be any more acceptable to instead add
a "fsl,num-gpio" property so that "ngpio" actually reported an accurate
value and non-existent GPIO pins couldn't be used/exported?
With the patch as is, if "fsl,gpio-mask" is not given, the driver
defaults to enabling all 32 gpio pins. Would it be any better if I
respun the patch to only add the "fsl,gpio-mask" property for the
mpc8572, p2020, and mpc8379 boards which have less than 32 gpio pins and
document the dts property as optional?
Thanks,
Peter
^ permalink raw reply
* Re: [PATCH] powerpc: mpc8xxx_gpio: Add ability to mask off unused GPIO pins
From: Anton Vorontsov @ 2009-12-05 17:56 UTC (permalink / raw)
To: Peter Tyser; +Cc: linuxppc-dev
In-Reply-To: <1259955820-28565-1-git-send-email-ptyser@xes-inc.com>
Hi Peter,
On Fri, Dec 04, 2009 at 01:43:40PM -0600, Peter Tyser wrote:
> This change resolves 2 issues:
> - Different chips have a different number of GPIO pins per controller.
> For example, the MPC8347 has 32, the P2020 16, and the mpc8572 8.
> Previously, the mpc8xxx_gpio driver assumed every chip had 32 GPIO
> pins which resulted in some processors reporting an incorrect 'ngpio'
> field in /sys. Additionally, users could export and "use" 32 GPIO
> pins, although in reality only a subset of the 32 pins had any real
> functionality.
>
> - Some boards don't utilize all available GPIO pins. Previously,
> unused GPIO pins could still be exported and "used", even though the
> pins had no real functionality. This is somewhat confusing to a user
> and also allow a user to do something "bad", like change an unused
> floating output into a floating input.
There are hundreds of other ways to screw things up.
Think of /dev/mem, you still able to change the registers.
Before changing any GPIO (whether it is a normal or reserved GPIO),
user has to consult with schematics/docs.
> Adding a new "fsl,gpio-mask" device tree property allows a dts file to
> accurately describe what GPIO pins are available for use on a given
> board.
I don't see any real usage for this. If device tree specifies a wrong
gpio in the gpios = <> property, then it's a bug in the device tree
and should be fixed (or workarounded in the platform code).
If a user fiddles with unknown gpios via sysfs interface, then it's
user's problem.
FWIW, we don't have any masks for reserved IRQs.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH/REPOST] net/mpc5200: Fix locking on fec_mpc52xx driver
From: Grant Likely @ 2009-12-05 4:33 UTC (permalink / raw)
To: netdev, linuxppc-dev, davem; +Cc: Asier Llano
From: Asier Llano Palacios <asierllano@gmail.com>
Fix the locking scheme on the fec_mpc52xx driver. This device can
receive IRQs from three sources; the FEC itself, the tx DMA, and the
rx DMA. Mutual exclusion was handled by taking a spin_lock() in the
critical regions, but because the handlers are run with IRQs enabled,
spin_lock() is insufficient and the driver can end up interrupting
a critical region anyway from another IRQ.
Asier Llano discovered that this occurs when an error IRQ is raised
in the middle of handling rx irqs which resulted in an sk_buff memory
leak.
In addition, locking is spotty at best in the driver and inspection
revealed quite a few places with insufficient locking.
This patch is based on Asier's initial work, but reworks a number of
things so that locks are held for as short a time as possible, so
that spin_lock_irqsave() is used everywhere, and so the locks are
dropped when calling into the network stack (because the lock only
protects the hardware interface; not the network stack).
Boot tested on a lite5200 with an NFS root. Has not been performance
tested.
Signed-off-by: Asier Llano <a.llano@ziv.es>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
Reposting due to messing up both subject line and davem's email addr.
drivers/net/fec_mpc52xx.c | 121 +++++++++++++++++++++++----------------------
1 files changed, 62 insertions(+), 59 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 66dace6..4889b4d 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -85,11 +85,15 @@ MODULE_PARM_DESC(debug, "debugging messages level");
static void mpc52xx_fec_tx_timeout(struct net_device *dev)
{
+ struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ unsigned long flags;
+
dev_warn(&dev->dev, "transmit timed out\n");
+ spin_lock_irqsave(&priv->lock, flags);
mpc52xx_fec_reset(dev);
-
dev->stats.tx_errors++;
+ spin_unlock_irqrestore(&priv->lock, flags);
netif_wake_queue(dev);
}
@@ -135,28 +139,32 @@ static void mpc52xx_fec_free_rx_buffers(struct net_device *dev, struct bcom_task
}
}
+static void
+mpc52xx_fec_rx_submit(struct net_device *dev, struct sk_buff *rskb)
+{
+ struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ struct bcom_fec_bd *bd;
+
+ bd = (struct bcom_fec_bd *) bcom_prepare_next_buffer(priv->rx_dmatsk);
+ bd->status = FEC_RX_BUFFER_SIZE;
+ bd->skb_pa = dma_map_single(dev->dev.parent, rskb->data,
+ FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
+ bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
+}
+
static int mpc52xx_fec_alloc_rx_buffers(struct net_device *dev, struct bcom_task *rxtsk)
{
- while (!bcom_queue_full(rxtsk)) {
- struct sk_buff *skb;
- struct bcom_fec_bd *bd;
+ struct sk_buff *skb;
+ while (!bcom_queue_full(rxtsk)) {
skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
- if (skb == NULL)
+ if (!skb)
return -EAGAIN;
/* zero out the initial receive buffers to aid debugging */
memset(skb->data, 0, FEC_RX_BUFFER_SIZE);
-
- bd = (struct bcom_fec_bd *)bcom_prepare_next_buffer(rxtsk);
-
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
-
- bcom_submit_next_buffer(rxtsk, skb);
+ mpc52xx_fec_rx_submit(dev, skb);
}
-
return 0;
}
@@ -328,13 +336,12 @@ static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
DMA_TO_DEVICE);
bcom_submit_next_buffer(priv->tx_dmatsk, skb);
+ spin_unlock_irqrestore(&priv->lock, flags);
if (bcom_queue_full(priv->tx_dmatsk)) {
netif_stop_queue(dev);
}
- spin_unlock_irqrestore(&priv->lock, flags);
-
return NETDEV_TX_OK;
}
@@ -359,9 +366,9 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ unsigned long flags;
- spin_lock(&priv->lock);
-
+ spin_lock_irqsave(&priv->lock, flags);
while (bcom_buffer_done(priv->tx_dmatsk)) {
struct sk_buff *skb;
struct bcom_fec_bd *bd;
@@ -372,11 +379,10 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
dev_kfree_skb_irq(skb);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
netif_wake_queue(dev);
- spin_unlock(&priv->lock);
-
return IRQ_HANDLED;
}
@@ -384,67 +390,60 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ struct sk_buff *rskb; /* received sk_buff */
+ struct sk_buff *skb; /* new sk_buff to enqueue in its place */
+ struct bcom_fec_bd *bd;
+ u32 status, physaddr;
+ int length;
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
while (bcom_buffer_done(priv->rx_dmatsk)) {
- struct sk_buff *skb;
- struct sk_buff *rskb;
- struct bcom_fec_bd *bd;
- u32 status;
rskb = bcom_retrieve_buffer(priv->rx_dmatsk, &status,
- (struct bcom_bd **)&bd);
- dma_unmap_single(dev->dev.parent, bd->skb_pa, rskb->len,
- DMA_FROM_DEVICE);
+ (struct bcom_bd **)&bd);
+ physaddr = bd->skb_pa;
/* Test for errors in received frame */
if (status & BCOM_FEC_RX_BD_ERRORS) {
/* Drop packet and reuse the buffer */
- bd = (struct bcom_fec_bd *)
- bcom_prepare_next_buffer(priv->rx_dmatsk);
-
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent,
- rskb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
-
- bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
-
+ mpc52xx_fec_rx_submit(dev, rskb);
dev->stats.rx_dropped++;
-
continue;
}
/* skbs are allocated on open, so now we allocate a new one,
* and remove the old (with the packet) */
skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
- if (skb) {
- /* Process the received skb */
- int length = status & BCOM_FEC_RX_BD_LEN_MASK;
-
- skb_put(rskb, length - 4); /* length without CRC32 */
-
- rskb->dev = dev;
- rskb->protocol = eth_type_trans(rskb, dev);
-
- netif_rx(rskb);
- } else {
+ if (!skb) {
/* Can't get a new one : reuse the same & drop pkt */
- dev_notice(&dev->dev, "Memory squeeze, dropping packet.\n");
+ dev_notice(&dev->dev, "Low memory - dropped packet.\n");
+ mpc52xx_fec_rx_submit(dev, rskb);
dev->stats.rx_dropped++;
-
- skb = rskb;
+ continue;
}
- bd = (struct bcom_fec_bd *)
- bcom_prepare_next_buffer(priv->rx_dmatsk);
+ /* Enqueue the new sk_buff back on the hardware */
+ mpc52xx_fec_rx_submit(dev, skb);
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
+ /* Process the received skb - Drop the spin lock while
+ * calling into the network stack */
+ spin_unlock_irqrestore(&priv->lock, flags);
- bcom_submit_next_buffer(priv->rx_dmatsk, skb);
+ dma_unmap_single(dev->dev.parent, physaddr, rskb->len,
+ DMA_FROM_DEVICE);
+ length = status & BCOM_FEC_RX_BD_LEN_MASK;
+ skb_put(rskb, length - 4); /* length without CRC32 */
+ rskb->dev = dev;
+ rskb->protocol = eth_type_trans(rskb, dev);
+ netif_rx(rskb);
+
+ spin_lock_irqsave(&priv->lock, flags);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
+
return IRQ_HANDLED;
}
@@ -454,6 +453,7 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
struct mpc52xx_fec __iomem *fec = priv->fec;
u32 ievent;
+ unsigned long flags;
ievent = in_be32(&fec->ievent);
@@ -471,9 +471,10 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
if (net_ratelimit() && (ievent & FEC_IEVENT_XFIFO_ERROR))
dev_warn(&dev->dev, "FEC_IEVENT_XFIFO_ERROR\n");
+ spin_lock_irqsave(&priv->lock, flags);
mpc52xx_fec_reset(dev);
+ spin_unlock_irqrestore(&priv->lock, flags);
- netif_wake_queue(dev);
return IRQ_HANDLED;
}
@@ -768,6 +769,8 @@ static void mpc52xx_fec_reset(struct net_device *dev)
bcom_enable(priv->tx_dmatsk);
mpc52xx_fec_start(dev);
+
+ netif_wake_queue(dev);
}
^ permalink raw reply related
* Re: [PATCH] MAINTAINERS: Add PowerPC patterns
From: Olof Johansson @ 2009-12-05 2:59 UTC (permalink / raw)
To: Joe Perches
Cc: linuxppc-dev, LKML, Jean Delvare, Paul Mackerras, Colin Leroy,
Darrick J. Wong
In-Reply-To: <1259947019.22783.116.camel@Joe-Laptop.home>
On Fri, Dec 04, 2009 at 09:16:59AM -0800, Joe Perches wrote:
> On Fri, 2009-12-04 at 20:59 +1100, Benjamin Herrenschmidt wrote:
> > On Fri, 2009-12-04 at 10:34 +0100, Jean Delvare wrote:
> > > I've sent it to linuxppc-dev@ozlabs.org on October 14th. This is the
> > > address which is listed 22 times in MAINTAINERS. If it isn't correct,
> > > then please update MAINTAINERS.
> > No it's fine both shoul work. Your patches are there, just waiting for
> > me to pick them up, I was just firing a reminder to the rest of the CC
> > list :-) (and I do remember fwd'ing a couple of your patches to the
> > list, for some reason they didn't make it to patchwork back then, that
> > was a few month ago).
> > Anyways, I've been stretched thin with all sort of stuff lately, so bear
> > with me if I'm a bit slow at taking or testing stuff, I'm doing my best.
>
> Adding patterns to the PowerPC sections of MAINTAINERS is useful.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Olof Johansson <olof@lixom.net>
There are a few other files under arch/powerpc that's not under platforms
that belongs to the pasemi code but it's not a big deal to not include
them. This goes most of the way.
-Olof
^ permalink raw reply
* Re: MPC8343EA Linux DTS file
From: Kim Phillips @ 2009-12-05 1:47 UTC (permalink / raw)
To: Junita Ajith; +Cc: linuxppc-dev
In-Reply-To: <72d214170912041543g207837a5v67c9bb157e0a2c8b@mail.gmail.com>
On Fri, 4 Dec 2009 15:43:18 -0800
Junita Ajith <ajijuni@gmail.com> wrote:
please keep the list on the cc:, and don't top-post.
> This is the entire DTS file:
> /dts-v1/;
>
> / {
> model = "SC3000";
> compatible = "MPC8349EMDS", "MPC834xMDS", "MPC83xxMDS";
you don't mention the u-boot and kernel version numbers: does your
linux kernel check for these compatibles? If yes, then use a debugger
to find out exactly where else the kernel could be dying.
Kim
^ permalink raw reply
* Re: MPC8343EA Linux DTS file
From: Kim Phillips @ 2009-12-04 23:39 UTC (permalink / raw)
To: agnel; +Cc: linuxppc-dev
In-Reply-To: <26645258.post@talk.nabble.com>
On Fri, 4 Dec 2009 14:56:25 -0800
agnel <ajijuni@gmail.com> wrote:
> We have an MPC8343EA based custom board.
>
> I am not able to get Linux up and running in this. No serial output to debug
> further.
> U-boot shows correct 'bdinfo' & 'clocks' output.
>
> I doubt the DTS file in Linux. anyone has DTS file for MPC8343??
>
> My current DTS file
well, for one, the dts you provide doesn't include a node for the ipic
interrupt controller.
Kim
^ permalink raw reply
* MPC8343EA Linux DTS file
From: agnel @ 2009-12-04 22:56 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <47F73625.9040903@matrix-vision.de>
Hi
We have an MPC8343EA based custom board.
I am not able to get Linux up and running in this. No serial output to debug
further.
U-boot shows correct 'bdinfo' & 'clocks' output.
I doubt the DTS file in Linux. anyone has DTS file for MPC8343??
My current DTS file
/dts-v1/;
/ {
model = "MPC8343";
compatible = "MPC8349EMDS", "MPC834xMDS", "MPC83xxMDS";
linux,phandle = <0x100>;
#size-cells = <0x1>;
#address-cells = <0x1>;
aliases {
ethernet0 = &enet0;
ethernet1 = &enet1;
serial0 = &serial0;
serial1 = &serial1;
pci0 = &pci0;
pci1 = &pci1;
};
cpus {
linux,phandle = <0x200>;
#cpus = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
PowerPC,8343EA@0 {
device_type = "cpu";
reg = <0x0>;
d-cache-line-size = <20>;
i-cache-line-size = <20>;
d-cache-size = <8000>;
i-cache-size = <8000>;
timebase-frequency = <0>; // from bootloader
bus-frequency = <0>; // from bootloader
clock-frequency = <0>; // from bootloader
32-bit;
};
};
memory {
device_type = "memory";
reg = <0x00000000 0x10000000>; // 256MB at 0
};
bcsr@e2400000 {
device_type = "board-control";
reg = <0xe2400000 0x8000>;
};
soc8343@e0000000 {
bus-frequency = <0x1>;
reg = <0xe0000000 0x200>;
ranges = <0x0 0xe0000000 0x100000>;
device_type = "soc";
#interrupt-cells = <0x2>;
#size-cells = <0x1>;
#address-cells = <0x1>;
wdt@200 {
device_type = "watchdog";
compatible = "mpc83xx_wdt";
reg = <0x200 0x100>;
};
i2c@3000 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl-i2c";
reg = <0x3000 0x100>;
interrupts = <14 0x8>;
interrupt-parent = <&ipic>;
dfsrr;
rtc@68 {
compatible = "dallas,ds1374";
reg = <0x68>;
};
};
i2c@3100 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <1>;
compatible = "fsl-i2c";
reg = <0x3100 0x100>;
interrupts = <15 0x8>;
interrupt-parent = <&ipic>;
dfsrr;
};
spi@7000 {
cell-index = <0>;
compatible = "fsl,spi";
reg = <0x7000 0x1000>;
interrupts = <16 0x8>;
interrupt-parent = <&ipic>;
mode = "cpu";
};
dma@82a8 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
reg = <0x82a8 4>;
ranges = <0 0x8100 0x1a8>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
cell-index = <0>;
dma-channel@0 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@80 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x80 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@100 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x100 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@180 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x180 0x28>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
};
/* phy type (ULPI or SERIAL) are only types supported for MPH */
/* port = 0 or 1 */
usb@22000 {
compatible = "fsl-usb2-mph";
reg = <0x22000 0x1000>;
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&ipic>;
interrupts = <39 0x8>;
phy_type = "ulpi";
port1;
};
/* phy type (ULPI, UTMI, UTMI_WIDE, SERIAL) */
usb@23000 {
compatible = "fsl-usb2-dr";
reg = <0x23000 0x1000>;
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&ipic>;
interrupts = <38 0x8>;
dr_mode = "otg";
phy_type = "ulpi";
};
mdio@24520 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,gianfar-mdio";
reg = <0x24520 0x20>;
phy0: ethernet-phy@0 {
interrupt-parent = <&ipic>;
interrupts = <17 0x8>;
reg = <0x0>;
device_type = "ethernet-phy";
};
phy1: ethernet-phy@1 {
interrupt-parent = <&ipic>;
interrupts = <18 0x8>;
reg = <0x1>;
device_type = "ethernet-phy";
};
enet0: ethernet@24000 {
cell-index = <0>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <0x24000 0x1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <32 0x8 33 0x8 34 0x8>;
interrupt-parent = <&ipic>;
phy-handle = <&phy0>;
linux,network-index = <0>;
};
enet1: ethernet@25000 {
cell-index = <1>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <0x25000 0x1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <35 0x8 36 0x8 37 0x8>;
interrupt-parent = <&ipic>;
phy-handle = <&phy1>;
linux,network-index = <1>;
};
serial0: serial@4500 {
cell-index = <0>;
device_type = "serial";
compatible = "ns16550";
reg = <0x4500 0x100>;
clock-frequency = <0>;
interrupts = <9 0x8>;
interrupt-parent = <&ipic>;
};
serial1: serial@4600 {
cell-index = <1>;
device_type = "serial";
compatible = "ns16550";
reg = <0x4600 0x100>;
clock-frequency = <0>;
interrupts = <10 0x8>;
interrupt-parent = <&ipic>;
};
crypto@30000 {
compatible = "fsl,sec2.0";
reg = <0x30000 0x10000>;
interrupts = <11 0x8>;
interrupt-parent = <&ipic>;
fsl,num-channels = <4>;
fsl,channel-fifo-len = <24>;
fsl,exec-units-mask = <0x7e>;
fsl,descriptor-types-mask = <0x01010ebf>;
};
Thanks,
agnel
--
View this message in context: http://old.nabble.com/MPC8343---%22unable-to-handle-paging-request-%40-0%22-tp16510741p26645258.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* MPC8343EA Linux DTS file
From: agnel @ 2009-12-04 22:55 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <47F73625.9040903@matrix-vision.de>
Hi
We have an MPC8343EA based custom board.
I am not able to get Linux up and running in this. No serial output to debug
further.
U-boot shows correct 'bdinfo' & 'clocks' output.
I doubt the DTS file in Linux. anyone has DTS file for MPC8343??
My current DTS file
/dts-v1/;
/ {
model = "Silvus-SC3000";
compatible = "MPC8349EMDS", "MPC834xMDS", "MPC83xxMDS";
linux,phandle = <0x100>;
#size-cells = <0x1>;
#address-cells = <0x1>;
aliases {
ethernet0 = &enet0;
ethernet1 = &enet1;
serial0 = &serial0;
serial1 = &serial1;
pci0 = &pci0;
pci1 = &pci1;
};
cpus {
linux,phandle = <0x200>;
#cpus = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
PowerPC,8343EA@0 {
device_type = "cpu";
reg = <0x0>;
d-cache-line-size = <20>;
i-cache-line-size = <20>;
d-cache-size = <8000>;
i-cache-size = <8000>;
timebase-frequency = <0>; // from bootloader
bus-frequency = <0>; // from bootloader
clock-frequency = <0>; // from bootloader
32-bit;
};
};
memory {
device_type = "memory";
reg = <0x00000000 0x10000000>; // 256MB at 0
};
bcsr@e2400000 {
device_type = "board-control";
reg = <0xe2400000 0x8000>;
};
soc8343@e0000000 {
bus-frequency = <0x1>;
reg = <0xe0000000 0x200>;
ranges = <0x0 0xe0000000 0x100000>;
device_type = "soc";
#interrupt-cells = <0x2>;
#size-cells = <0x1>;
#address-cells = <0x1>;
wdt@200 {
device_type = "watchdog";
compatible = "mpc83xx_wdt";
reg = <0x200 0x100>;
};
i2c@3000 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl-i2c";
reg = <0x3000 0x100>;
interrupts = <14 0x8>;
interrupt-parent = <&ipic>;
dfsrr;
rtc@68 {
compatible = "dallas,ds1374";
reg = <0x68>;
};
};
i2c@3100 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <1>;
compatible = "fsl-i2c";
reg = <0x3100 0x100>;
interrupts = <15 0x8>;
interrupt-parent = <&ipic>;
dfsrr;
};
spi@7000 {
cell-index = <0>;
compatible = "fsl,spi";
reg = <0x7000 0x1000>;
interrupts = <16 0x8>;
interrupt-parent = <&ipic>;
mode = "cpu";
};
dma@82a8 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
reg = <0x82a8 4>;
ranges = <0 0x8100 0x1a8>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
cell-index = <0>;
dma-channel@0 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@80 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x80 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@100 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x100 0x80>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
dma-channel@180 {
compatible = "fsl,mpc8349-dma-channel",
"fsl,elo-dma-channel";
reg = <0x180 0x28>;
interrupt-parent = <&ipic>;
interrupts = <71 8>;
};
};
/* phy type (ULPI or SERIAL) are only types supported for MPH */
/* port = 0 or 1 */
usb@22000 {
compatible = "fsl-usb2-mph";
reg = <0x22000 0x1000>;
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&ipic>;
interrupts = <39 0x8>;
phy_type = "ulpi";
port1;
};
/* phy type (ULPI, UTMI, UTMI_WIDE, SERIAL) */
usb@23000 {
compatible = "fsl-usb2-dr";
reg = <0x23000 0x1000>;
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&ipic>;
interrupts = <38 0x8>;
dr_mode = "otg";
phy_type = "ulpi";
};
mdio@24520 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,gianfar-mdio";
reg = <0x24520 0x20>;
phy0: ethernet-phy@0 {
interrupt-parent = <&ipic>;
interrupts = <17 0x8>;
reg = <0x0>;
device_type = "ethernet-phy";
};
phy1: ethernet-phy@1 {
interrupt-parent = <&ipic>;
interrupts = <18 0x8>;
reg = <0x1>;
device_type = "ethernet-phy";
};
enet0: ethernet@24000 {
cell-index = <0>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <0x24000 0x1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <32 0x8 33 0x8 34 0x8>;
interrupt-parent = <&ipic>;
phy-handle = <&phy0>;
linux,network-index = <0>;
};
enet1: ethernet@25000 {
cell-index = <1>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <0x25000 0x1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <35 0x8 36 0x8 37 0x8>;
interrupt-parent = <&ipic>;
phy-handle = <&phy1>;
linux,network-index = <1>;
};
serial0: serial@4500 {
cell-index = <0>;
device_type = "serial";
compatible = "ns16550";
reg = <0x4500 0x100>;
clock-frequency = <0>;
interrupts = <9 0x8>;
interrupt-parent = <&ipic>;
};
serial1: serial@4600 {
cell-index = <1>;
device_type = "serial";
compatible = "ns16550";
reg = <0x4600 0x100>;
clock-frequency = <0>;
interrupts = <10 0x8>;
interrupt-parent = <&ipic>;
};
crypto@30000 {
compatible = "fsl,sec2.0";
reg = <0x30000 0x10000>;
interrupts = <11 0x8>;
interrupt-parent = <&ipic>;
fsl,num-channels = <4>;
fsl,channel-fifo-len = <24>;
fsl,exec-units-mask = <0x7e>;
fsl,descriptor-types-mask = <0x01010ebf>;
};
Thanks,
agnel
--
View this message in context: http://old.nabble.com/MPC8343---%22unable-to-handle-paging-request-%40-0%22-tp16510741p26645256.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [v10 PATCH 2/9]: cpuidle: cleanup drivers/cpuidle/cpuidle.c
From: Torsten Duwe @ 2009-12-04 22:20 UTC (permalink / raw)
To: arun
Cc: linux-arch, Peter Zijlstra, Venkatesh Pallipadi, linux-kernel,
linux-acpi, Ingo Molnar, linuxppc-dev
In-Reply-To: <20091202095705.GC27251@linux.vnet.ibm.com>
On Wednesday 02 December 2009, Arun R Bharadwaj wrote:
> * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-12-02 15:24:27]:
>
> This patch cleans up drivers/cpuidle/cpuidle.c
> Earlier cpuidle assumed pm_idle as the default idle loop. Break that
> assumption and make it more generic.
Is there a problem with the old pm_idle? Couldn't it be integrated more
transparently, instead of replacing it this intrusively?
> --- linux.trees.git.orig/include/linux/cpuidle.h
> +++ linux.trees.git/include/linux/cpuidle.h
> @@ -41,7 +41,7 @@ struct cpuidle_state {
> unsigned long long usage;
> unsigned long long time; /* in US */
>
> - int (*enter) (struct cpuidle_device *dev,
> + void (*enter) (struct cpuidle_device *dev,
> struct cpuidle_state *state);
> };
While it may be a good idea to move the residency calculation to one central
place, at least in theory a cpuidle_state->enter() function could have a
better method to determine its value.
Either way you're implicitly introducing an API change here, and you're at
least missing two functions on ARM and SuperH, respectively. Could you
separate this API change out, and not take it for granted in the other
patches?
Torsten
^ permalink raw reply
* Re: TQM5200 + SM501 FB
From: Wolfgang Denk @ 2009-12-04 22:04 UTC (permalink / raw)
To: Rolf Offermanns; +Cc: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]
Dear Rolf Offermanns,
In message <4B191B54.6030804@sysgo.com> you wrote:
>
> does anyone has a working SM501 framebuffer on the STK5200 board? The
> board itself works fine on the 2.6.32 kernel.
This is a pretty well supported configuration.
> I got a patch from TQ which used to work around 2.6.24 but it seems with
> the unification of the "simple mpc5200" boards the sm501 support got lost.
Don't expect any real support from TQ on this; so far they never
spent any efforts on the arch/powerpc port of the TQM5200.
> I toyed around with the patch a bit and I got the driver to probe and
> initialize the chip, but I don't get an output on the crt port and an
> open() on /dev/fb0 or /dev/fb1 fails with "no device".
Did you check if you eventually have forgotten to enable Framebuffer
console in your kernel configuration? Make sure to set:
Device Drivers --->
Character devices --->
[*] Virtual terminal
[*] Support for console on virtual terminal (NEW)
...
Multifunction device drivers --->
<*> Support for Silicon Motion SM501
...
Graphics support --->
<*> Support for frame buffer devices --->
[*] Framebuffer foreign endianness support --->
Choice endianness support (Support for Big- and Little-Endian framebuffers) --->
...
<*> Silicon Motion SM501 framebuffer support
...
Console display driver support --->
[ ] VGA text console (! disable VGA text console!)
<*> Framebuffer Console support
Please find attached below a patch which fixes some issues (but we
don't consider it clean enough for inclusion into mainline yet), and a
known to be working config file. [Kudos to Anatolij for providing
this.]
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
What was sliced bread the greatest thing since?
[-- Attachment #2: 0001-sm501-support-SM501-on-TQM5200.patch --]
[-- Type: application/x-patch , Size: 3845 bytes --]
[-- Attachment #3: config-tqm5200-sm501.gz --]
[-- Type: application/x-gzip , Size: 9514 bytes --]
^ permalink raw reply
* Re: using different format for hugetlbfs
From: Benjamin Herrenschmidt @ 2009-12-04 21:25 UTC (permalink / raw)
To: Kumar Gala; +Cc: linux-ppc list, David Gibson
In-Reply-To: <90D0766D-30A2-4ABE-9707-C7F64A697BFE@kernel.crashing.org>
On Fri, 2009-12-04 at 08:09 -0600, Kumar Gala wrote:
> On Dec 4, 2009, at 2:58 AM, Benjamin Herrenschmidt wrote:
>
> > On Fri, 2009-12-04 at 01:18 -0600, Kumar Gala wrote:
> >> Ben, David,
> >>
> >> If we want to support true 4G/4G split on ppc32 using the MSB of the
> >> address to determine of the pgd_t is for hugetlbfs isn't going to
> >> work. Since every pointer in the pgd_t -> pud_t -> pmd_t is point to
> >> at least a 4K page I would think the low order 12-bits should always
> >> be 0.
> >
> > On 32 bit maybe. On 64, the pg/u/md's can be smaller. I don't really
> > want to have a different encoding for both types though.
>
> What do you mean they can be smaller? We have some scenario when we
> dont allocate a full page? I agree having the encodings be different
> would be bad. I'm trying to avoid having it be different between 32
> bit and 64 (but maybe that will be impossible).
Yes. The intermediary levels are smaller on 64-bit. Also, with hugetlbfs
it can create special levels of various sizes depending on the
requirements to fit a given huge page size. And that would be true of
both 32 and 64-bit in fact.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
From: Grant Likely @ 2009-12-04 21:24 UTC (permalink / raw)
To: netdev, linuxppc-dev, David S. Miller; +Cc: Asier Llano
In-Reply-To: <20091204211737.26998.58504.stgit@angua>
Oops, sorry about the messed up subject. that was sloppy. Real
subject should be:
net/mpc5200: Fix locking on fec_mpc52xx driver
On Fri, Dec 4, 2009 at 2:20 PM, Grant Likely <grant.likely@secretlab.ca> wr=
ote:
> From: Asier Llano Palacios <asierllano@gmail.com>
>
> net/mpc5200: Fix locking on fec_mpc52xx driver
>
> Fix the locking scheme on the fec_mpc52xx driver. =A0This device can
> receive IRQs from three sources; the FEC itself, the tx DMA, and the
> rx DMA. =A0Mutual exclusion was handled by taking a spin_lock() in the
> critical regions, but because the handlers are run with IRQs enabled,
> spin_lock() is insufficient and the driver can end up interrupting
> a critical region anyway from another IRQ.
>
> Asier Llano discovered that this occurs when an error IRQ is raised
> in the middle of handling rx irqs which resulted in an sk_buff memory
> leak.
>
> In addition, locking is spotty at best in the driver and inspection
> revealed quite a few places with insufficient locking.
>
> This patch is based on Asier's initial work, but reworks a number of
> things so that locks are held for as short a time as possible, so
> that spin_lock_irqsave() is used everywhere, and so the locks are
> dropped when calling into the network stack (because the lock only
> protects the hardware interface; not the network stack).
>
> Boot tested on a lite5200 with an NFS root. =A0Has not been performance
> tested.
>
> Signed-off-by: Asier Llano <a.llano@ziv.es>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> Asier, can you please test this? =A0It took me a while to respond to
> your initial post because I was concerned about some of the latency
> issues, and I was concerned about disabling IRQs for long periods in
> the RX handler. =A0I think it should be good now, but it needs testing.
>
> Cheers,
> g.
>
> =A0drivers/net/fec_mpc52xx.c | =A0121 +++++++++++++++++++++++------------=
----------
> =A01 files changed, 62 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index 66dace6..4889b4d 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -85,11 +85,15 @@ MODULE_PARM_DESC(debug, "debugging messages level");
>
> =A0static void mpc52xx_fec_tx_timeout(struct net_device *dev)
> =A0{
> + =A0 =A0 =A0 struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> + =A0 =A0 =A0 unsigned long flags;
> +
> =A0 =A0 =A0 =A0dev_warn(&dev->dev, "transmit timed out\n");
>
> + =A0 =A0 =A0 spin_lock_irqsave(&priv->lock, flags);
> =A0 =A0 =A0 =A0mpc52xx_fec_reset(dev);
> -
> =A0 =A0 =A0 =A0dev->stats.tx_errors++;
> + =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
>
> =A0 =A0 =A0 =A0netif_wake_queue(dev);
> =A0}
> @@ -135,28 +139,32 @@ static void mpc52xx_fec_free_rx_buffers(struct net_=
device *dev, struct bcom_task
> =A0 =A0 =A0 =A0}
> =A0}
>
> +static void
> +mpc52xx_fec_rx_submit(struct net_device *dev, struct sk_buff *rskb)
> +{
> + =A0 =A0 =A0 struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> + =A0 =A0 =A0 struct bcom_fec_bd *bd;
> +
> + =A0 =A0 =A0 bd =3D (struct bcom_fec_bd *) bcom_prepare_next_buffer(priv=
->rx_dmatsk);
> + =A0 =A0 =A0 bd->status =3D FEC_RX_BUFFER_SIZE;
> + =A0 =A0 =A0 bd->skb_pa =3D dma_map_single(dev->dev.parent, rskb->data,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FEC=
_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> + =A0 =A0 =A0 bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
> +}
> +
> =A0static int mpc52xx_fec_alloc_rx_buffers(struct net_device *dev, struct=
bcom_task *rxtsk)
> =A0{
> - =A0 =A0 =A0 while (!bcom_queue_full(rxtsk)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sk_buff *skb;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct bcom_fec_bd *bd;
> + =A0 =A0 =A0 struct sk_buff *skb;
>
> + =A0 =A0 =A0 while (!bcom_queue_full(rxtsk)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0skb =3D dev_alloc_skb(FEC_RX_BUFFER_SIZE);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (skb =3D=3D NULL)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!skb)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EAGAIN;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* zero out the initial receive buffers to=
aid debugging */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memset(skb->data, 0, FEC_RX_BUFFER_SIZE);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd =3D (struct bcom_fec_bd *)bcom_prepare_n=
ext_buffer(rxtsk);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D FEC_RX_BUFFER_SIZE;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->skb_pa =3D dma_map_single(dev->dev.pare=
nt, skb->data,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FEC_RX_BUFF=
ER_SIZE, DMA_FROM_DEVICE);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_submit_next_buffer(rxtsk, skb);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_fec_rx_submit(dev, skb);
> =A0 =A0 =A0 =A0}
> -
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> @@ -328,13 +336,12 @@ static int mpc52xx_fec_start_xmit(struct sk_buff *s=
kb, struct net_device *dev)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DM=
A_TO_DEVICE);
>
> =A0 =A0 =A0 =A0bcom_submit_next_buffer(priv->tx_dmatsk, skb);
> + =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
>
> =A0 =A0 =A0 =A0if (bcom_queue_full(priv->tx_dmatsk)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0netif_stop_queue(dev);
> =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
> -
> =A0 =A0 =A0 =A0return NETDEV_TX_OK;
> =A0}
>
> @@ -359,9 +366,9 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, =
void *dev_id)
> =A0{
> =A0 =A0 =A0 =A0struct net_device *dev =3D dev_id;
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> + =A0 =A0 =A0 unsigned long flags;
>
> - =A0 =A0 =A0 spin_lock(&priv->lock);
> -
> + =A0 =A0 =A0 spin_lock_irqsave(&priv->lock, flags);
> =A0 =A0 =A0 =A0while (bcom_buffer_done(priv->tx_dmatsk)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct sk_buff *skb;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct bcom_fec_bd *bd;
> @@ -372,11 +379,10 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq=
, void *dev_id)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_kfree_skb_irq(skb);
> =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
>
> =A0 =A0 =A0 =A0netif_wake_queue(dev);
>
> - =A0 =A0 =A0 spin_unlock(&priv->lock);
> -
> =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0}
>
> @@ -384,67 +390,60 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq=
, void *dev_id)
> =A0{
> =A0 =A0 =A0 =A0struct net_device *dev =3D dev_id;
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> + =A0 =A0 =A0 struct sk_buff *rskb; /* received sk_buff */
> + =A0 =A0 =A0 struct sk_buff *skb; =A0/* new sk_buff to enqueue in its pl=
ace */
> + =A0 =A0 =A0 struct bcom_fec_bd *bd;
> + =A0 =A0 =A0 u32 status, physaddr;
> + =A0 =A0 =A0 int length;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&priv->lock, flags);
>
> =A0 =A0 =A0 =A0while (bcom_buffer_done(priv->rx_dmatsk)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sk_buff *skb;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sk_buff *rskb;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct bcom_fec_bd *bd;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 status;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rskb =3D bcom_retrieve_buffer(priv->rx_dma=
tsk, &status,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (struct bco=
m_bd **)&bd);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(dev->dev.parent, bd->skb_p=
a, rskb->len,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DMA_FROM=
_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 (struct bcom_bd **)&bd);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 physaddr =3D bd->skb_pa;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Test for errors in received frame */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (status & BCOM_FEC_RX_BD_ERRORS) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Drop packet and reuse t=
he buffer */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd =3D (struct bcom_fec_bd =
*)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_prepar=
e_next_buffer(priv->rx_dmatsk);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D FEC_RX_BUFFE=
R_SIZE;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->skb_pa =3D dma_map_sing=
le(dev->dev.parent,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 rskb->data,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_submit_next_buffer(pri=
v->rx_dmatsk, rskb);
> -
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_fec_rx_submit(dev, =
rskb);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev->stats.rx_dropped++;
> -
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* skbs are allocated on open, so now we a=
llocate a new one,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * and remove the old (with the packet) */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0skb =3D dev_alloc_skb(FEC_RX_BUFFER_SIZE);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (skb) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Process the received skb=
*/
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int length =3D status & BCO=
M_FEC_RX_BD_LEN_MASK;
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 skb_put(rskb, length - 4); =
=A0 =A0 =A0/* length without CRC32 */
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rskb->dev =3D dev;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rskb->protocol =3D eth_type=
_trans(rskb, dev);
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 netif_rx(rskb);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!skb) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Can't get a new one : r=
euse the same & drop pkt */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_notice(&dev->dev, "Memo=
ry squeeze, dropping packet.\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_notice(&dev->dev, "Low =
memory - dropped packet.\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_fec_rx_submit(dev, =
rskb);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev->stats.rx_dropped++;
> -
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 skb =3D rskb;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd =3D (struct bcom_fec_bd *)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_prepare_next_buffer(pr=
iv->rx_dmatsk);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Enqueue the new sk_buff back on the hard=
ware */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_fec_rx_submit(dev, skb);
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D FEC_RX_BUFFER_SIZE;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->skb_pa =3D dma_map_single(dev->dev.pare=
nt, skb->data,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FEC_RX_BUFF=
ER_SIZE, DMA_FROM_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Process the received skb - Drop the spin=
lock while
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* calling into the network stack */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_submit_next_buffer(priv->rx_dmatsk, sk=
b);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(dev->dev.parent, physaddr,=
rskb->len,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DMA_FROM=
_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 length =3D status & BCOM_FEC_RX_BD_LEN_MASK=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 skb_put(rskb, length - 4); =A0 =A0 =A0/* le=
ngth without CRC32 */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 rskb->dev =3D dev;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 rskb->protocol =3D eth_type_trans(rskb, dev=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 netif_rx(rskb);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&priv->lock, flags);
> =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
> +
> =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0}
>
> @@ -454,6 +453,7 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, voi=
d *dev_id)
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> =A0 =A0 =A0 =A0struct mpc52xx_fec __iomem *fec =3D priv->fec;
> =A0 =A0 =A0 =A0u32 ievent;
> + =A0 =A0 =A0 unsigned long flags;
>
> =A0 =A0 =A0 =A0ievent =3D in_be32(&fec->ievent);
>
> @@ -471,9 +471,10 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, vo=
id *dev_id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (net_ratelimit() && (ievent & FEC_IEVEN=
T_XFIFO_ERROR))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_warn(&dev->dev, "FEC_I=
EVENT_XFIFO_ERROR\n");
>
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&priv->lock, flags);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc52xx_fec_reset(dev);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&priv->lock, flags);
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 netif_wake_queue(dev);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0 =A0 =A0 =A0}
>
> @@ -768,6 +769,8 @@ static void mpc52xx_fec_reset(struct net_device *dev)
> =A0 =A0 =A0 =A0bcom_enable(priv->tx_dmatsk);
>
> =A0 =A0 =A0 =A0mpc52xx_fec_start(dev);
> +
> + =A0 =A0 =A0 netif_wake_queue(dev);
> =A0}
>
>
>
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH] Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
From: Grant Likely @ 2009-12-04 21:20 UTC (permalink / raw)
To: netdev, linuxppc-dev, David S. Miller; +Cc: Asier Llano
From: Asier Llano Palacios <asierllano@gmail.com>
net/mpc5200: Fix locking on fec_mpc52xx driver
Fix the locking scheme on the fec_mpc52xx driver. This device can
receive IRQs from three sources; the FEC itself, the tx DMA, and the
rx DMA. Mutual exclusion was handled by taking a spin_lock() in the
critical regions, but because the handlers are run with IRQs enabled,
spin_lock() is insufficient and the driver can end up interrupting
a critical region anyway from another IRQ.
Asier Llano discovered that this occurs when an error IRQ is raised
in the middle of handling rx irqs which resulted in an sk_buff memory
leak.
In addition, locking is spotty at best in the driver and inspection
revealed quite a few places with insufficient locking.
This patch is based on Asier's initial work, but reworks a number of
things so that locks are held for as short a time as possible, so
that spin_lock_irqsave() is used everywhere, and so the locks are
dropped when calling into the network stack (because the lock only
protects the hardware interface; not the network stack).
Boot tested on a lite5200 with an NFS root. Has not been performance
tested.
Signed-off-by: Asier Llano <a.llano@ziv.es>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
Asier, can you please test this? It took me a while to respond to
your initial post because I was concerned about some of the latency
issues, and I was concerned about disabling IRQs for long periods in
the RX handler. I think it should be good now, but it needs testing.
Cheers,
g.
drivers/net/fec_mpc52xx.c | 121 +++++++++++++++++++++++----------------------
1 files changed, 62 insertions(+), 59 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 66dace6..4889b4d 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -85,11 +85,15 @@ MODULE_PARM_DESC(debug, "debugging messages level");
static void mpc52xx_fec_tx_timeout(struct net_device *dev)
{
+ struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ unsigned long flags;
+
dev_warn(&dev->dev, "transmit timed out\n");
+ spin_lock_irqsave(&priv->lock, flags);
mpc52xx_fec_reset(dev);
-
dev->stats.tx_errors++;
+ spin_unlock_irqrestore(&priv->lock, flags);
netif_wake_queue(dev);
}
@@ -135,28 +139,32 @@ static void mpc52xx_fec_free_rx_buffers(struct net_device *dev, struct bcom_task
}
}
+static void
+mpc52xx_fec_rx_submit(struct net_device *dev, struct sk_buff *rskb)
+{
+ struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ struct bcom_fec_bd *bd;
+
+ bd = (struct bcom_fec_bd *) bcom_prepare_next_buffer(priv->rx_dmatsk);
+ bd->status = FEC_RX_BUFFER_SIZE;
+ bd->skb_pa = dma_map_single(dev->dev.parent, rskb->data,
+ FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
+ bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
+}
+
static int mpc52xx_fec_alloc_rx_buffers(struct net_device *dev, struct bcom_task *rxtsk)
{
- while (!bcom_queue_full(rxtsk)) {
- struct sk_buff *skb;
- struct bcom_fec_bd *bd;
+ struct sk_buff *skb;
+ while (!bcom_queue_full(rxtsk)) {
skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
- if (skb == NULL)
+ if (!skb)
return -EAGAIN;
/* zero out the initial receive buffers to aid debugging */
memset(skb->data, 0, FEC_RX_BUFFER_SIZE);
-
- bd = (struct bcom_fec_bd *)bcom_prepare_next_buffer(rxtsk);
-
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
-
- bcom_submit_next_buffer(rxtsk, skb);
+ mpc52xx_fec_rx_submit(dev, skb);
}
-
return 0;
}
@@ -328,13 +336,12 @@ static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
DMA_TO_DEVICE);
bcom_submit_next_buffer(priv->tx_dmatsk, skb);
+ spin_unlock_irqrestore(&priv->lock, flags);
if (bcom_queue_full(priv->tx_dmatsk)) {
netif_stop_queue(dev);
}
- spin_unlock_irqrestore(&priv->lock, flags);
-
return NETDEV_TX_OK;
}
@@ -359,9 +366,9 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ unsigned long flags;
- spin_lock(&priv->lock);
-
+ spin_lock_irqsave(&priv->lock, flags);
while (bcom_buffer_done(priv->tx_dmatsk)) {
struct sk_buff *skb;
struct bcom_fec_bd *bd;
@@ -372,11 +379,10 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
dev_kfree_skb_irq(skb);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
netif_wake_queue(dev);
- spin_unlock(&priv->lock);
-
return IRQ_HANDLED;
}
@@ -384,67 +390,60 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ struct sk_buff *rskb; /* received sk_buff */
+ struct sk_buff *skb; /* new sk_buff to enqueue in its place */
+ struct bcom_fec_bd *bd;
+ u32 status, physaddr;
+ int length;
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
while (bcom_buffer_done(priv->rx_dmatsk)) {
- struct sk_buff *skb;
- struct sk_buff *rskb;
- struct bcom_fec_bd *bd;
- u32 status;
rskb = bcom_retrieve_buffer(priv->rx_dmatsk, &status,
- (struct bcom_bd **)&bd);
- dma_unmap_single(dev->dev.parent, bd->skb_pa, rskb->len,
- DMA_FROM_DEVICE);
+ (struct bcom_bd **)&bd);
+ physaddr = bd->skb_pa;
/* Test for errors in received frame */
if (status & BCOM_FEC_RX_BD_ERRORS) {
/* Drop packet and reuse the buffer */
- bd = (struct bcom_fec_bd *)
- bcom_prepare_next_buffer(priv->rx_dmatsk);
-
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent,
- rskb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
-
- bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
-
+ mpc52xx_fec_rx_submit(dev, rskb);
dev->stats.rx_dropped++;
-
continue;
}
/* skbs are allocated on open, so now we allocate a new one,
* and remove the old (with the packet) */
skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
- if (skb) {
- /* Process the received skb */
- int length = status & BCOM_FEC_RX_BD_LEN_MASK;
-
- skb_put(rskb, length - 4); /* length without CRC32 */
-
- rskb->dev = dev;
- rskb->protocol = eth_type_trans(rskb, dev);
-
- netif_rx(rskb);
- } else {
+ if (!skb) {
/* Can't get a new one : reuse the same & drop pkt */
- dev_notice(&dev->dev, "Memory squeeze, dropping packet.\n");
+ dev_notice(&dev->dev, "Low memory - dropped packet.\n");
+ mpc52xx_fec_rx_submit(dev, rskb);
dev->stats.rx_dropped++;
-
- skb = rskb;
+ continue;
}
- bd = (struct bcom_fec_bd *)
- bcom_prepare_next_buffer(priv->rx_dmatsk);
+ /* Enqueue the new sk_buff back on the hardware */
+ mpc52xx_fec_rx_submit(dev, skb);
- bd->status = FEC_RX_BUFFER_SIZE;
- bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
- FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
+ /* Process the received skb - Drop the spin lock while
+ * calling into the network stack */
+ spin_unlock_irqrestore(&priv->lock, flags);
- bcom_submit_next_buffer(priv->rx_dmatsk, skb);
+ dma_unmap_single(dev->dev.parent, physaddr, rskb->len,
+ DMA_FROM_DEVICE);
+ length = status & BCOM_FEC_RX_BD_LEN_MASK;
+ skb_put(rskb, length - 4); /* length without CRC32 */
+ rskb->dev = dev;
+ rskb->protocol = eth_type_trans(rskb, dev);
+ netif_rx(rskb);
+
+ spin_lock_irqsave(&priv->lock, flags);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
+
return IRQ_HANDLED;
}
@@ -454,6 +453,7 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
struct mpc52xx_fec __iomem *fec = priv->fec;
u32 ievent;
+ unsigned long flags;
ievent = in_be32(&fec->ievent);
@@ -471,9 +471,10 @@ static irqreturn_t mpc52xx_fec_interrupt(int irq, void *dev_id)
if (net_ratelimit() && (ievent & FEC_IEVENT_XFIFO_ERROR))
dev_warn(&dev->dev, "FEC_IEVENT_XFIFO_ERROR\n");
+ spin_lock_irqsave(&priv->lock, flags);
mpc52xx_fec_reset(dev);
+ spin_unlock_irqrestore(&priv->lock, flags);
- netif_wake_queue(dev);
return IRQ_HANDLED;
}
@@ -768,6 +769,8 @@ static void mpc52xx_fec_reset(struct net_device *dev)
bcom_enable(priv->tx_dmatsk);
mpc52xx_fec_start(dev);
+
+ netif_wake_queue(dev);
}
^ permalink raw reply related
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