* Re: DTC sources move
From: Jon Loeliger @ 2009-03-27 18:55 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, monstr, devicetree-discuss, David Gibson
In-Reply-To: <fa686aa40903270857g7f1a3b64y93067cba5a7a6a28@mail.gmail.com>
> On Fri, Mar 27, 2009 at 8:16 AM, Michal Simek <monstr@monstr.eu> wrote:
> > Hi Ben,
> >
> > could you please move DTC to any generic location. I would like to use it f
> or Microblaze cpu too.
>
> Michal,
>
> David Gibson is the one responsible for the in-tree dtc source, not
> Ben, and you should prepare the patches yourself to move dtc.
And, I might add, please update it from its main repo in the process...
jdl
^ permalink raw reply
* Re: [PATCH] ucc_geth: Convert to net_device_ops
From: Timur Tabi @ 2009-03-27 18:50 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: leoli, linuxppc-dev, David Miller, netdev
In-Reply-To: <OF4B79F4DE.4DC6DEA7-ONC1257583.007ECBEA-C1257583.007F2805@transmode.se>
On Tue, Mar 24, 2009 at 6:08 PM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> + =A0 =A0 =A0 .ndo_change_mtu =A0 =A0 =A0 =A0 =3D eth_change_mtu,
I have an old patch for change_mtu support for ucc_geth which I've
been meaning to push upstream. This patch uses this function instead
of eth_change_mtu():
+static int ucc_geth_change_mtu(struct net_device *dev, int new_mtu)
+{
+ int max_rx_buf_length;
+ struct ucc_geth_private *ugeth =3D netdev_priv(dev);
+ int frame_size =3D new_mtu + 18;
+ struct ucc_geth_info *ug_info;
+ struct ucc_fast_info *uf_info;
+
+ if (ugeth->p_rx_glbl_pram) {
+ printk("%s: Interface is active!\n", __FUNCTION__);
+ } else {
+
+ ug_info =3D ugeth->ug_info;
+ uf_info =3D &ug_info->uf_info;
+
+ if ((frame_size < 64) || (frame_size > JUMBO_FRAME_SIZE)) {
+ printk(KERN_ERR "%s: Invalid MTU setting\n", dev->name);
+ return -EINVAL;
+ }
+
+ max_rx_buf_length =3D
+ (frame_size & ~(UCC_GETH_MRBLR_ALIGNMENT - 1)) +
+ UCC_GETH_MRBLR_ALIGNMENT;
+
+ ugeth->ug_info->uf_info.max_rx_buf_length =3D max_rx_buf_length;
+ dev->mtu =3D new_mtu;
+
+ /* Set the max. rx buffer size (MRBLR) */
+ uf_info->max_rx_buf_length =3D max_rx_buf_length;
+
+ /* set the max. frame length (MFLR) */
+ ug_info->maxFrameLength =3D frame_size;
+
+ ugeth_info("%s: MTU =3D %d (frame size=3D%d)\n", dev->name,
+ dev->mtu, frame_size);
+ }
+ return 0;
+}
Do you think this is how it should be done? I don't know enough about
ucc_geth and MTUs to know off-hand.
--=20
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* [PATCH] ucc_geth: Fix three oopses in PHY {de,}initialization code
From: Anton Vorontsov @ 2009-03-27 17:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Joakim Tjernlund, linuxppc-dev
When there are no free snums, UCC ethernet should gracefully fail, but
currently it oopses this way:
# ifconfig eth0 up
fill_init_enet_entries: Can not get SNUM.
ucc_geth_startup: Can not fill p_init_enet_param_shadow.
eth0: Cannot configure net device, aborting.
Unable to handle kernel paging request for data at address 0x00000190
Faulting instruction address: 0xc0294c88
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c0294c88] mutex_lock+0x0/0x1c
LR [c01b6be8] phy_stop+0x20/0x70
Call Trace:
[efb25da0] [efb2eb60] 0xefb2eb60 (unreliable)
[efb25db0] [c01b2058] ucc_geth_stop+0x2c/0x8c
[efb25dd0] [c01b4194] ucc_geth_open+0x48/0x27c
[efb25df0] [c020eec0] dev_open+0xc0/0x118
[...]
This is because the ucc_geth_stop() routine assumes that ugeth->phydev
is always initialized by the ucc_geth_open(), while it is not in case
of errors.
If we add a check to the ucc_geth_stop(), then another oops pops up:
Unable to handle kernel paging request for data at address 0x00000004
Faulting instruction address: 0xc01b46a4
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01b46a4] adjust_link+0x20/0x1b4
LR [c01b770c] phy_state_machine+0xdc/0x44c
Call Trace:
[ef83bf10] [c021b388] linkwatch_schedule_work+0x74/0xf8 (unreliable)
[ef83bf40] [c01b770c] phy_state_machine+0xdc/0x44c
[ef83bf60] [c004c13c] run_workqueue+0xb8/0x148
[ef83bf90] [c004c870] worker_thread+0x70/0xd0
[ef83bfd0] [c00505fc] kthread+0x48/0x84
[ef83bff0] [c000f464] kernel_thread+0x4c/0x68
[...]
That one happens because ucc_geth_stop() does not call phy_disconnect()
and so phylib state machine is running without any idea that a MAC has
just died.
Also, when device tree specifies fixed-link, and CONFIG_FIXED_PHY
is disabled, we'll get this oops:
0:01 not found
eth2: Could not attach to PHY
eth2: Cannot initialize PHY, aborting.
Unable to handle kernel paging request for data at address 0x00000190
Faulting instruction address: 0xc02967d0
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c02967d0] mutex_lock+0x0/0x1c
LR [c01b6bcc] phy_stop+0x20/0x70
Call Trace:
[ef82be50] [efb6bb60] 0xefb6bb60 (unreliable)
[ef82be60] [c01b2058] ucc_geth_stop+0x2c/0x8c
[ef82be80] [c01b4194] ucc_geth_open+0x48/0x27c
[ef82bea0] [c0210a04] dev_open+0xc0/0x118
[ef82bec0] [c020f85c] dev_change_flags+0x84/0x1ac
[ef82bee0] [c037b768] ic_open_devs+0x168/0x2bc
[ef82bf20] [c037ca98] ip_auto_config+0x90/0x28c
[ef82bf60] [c0001b9c] do_one_initcall+0x34/0x1a0
[ef82bfd0] [c035e240] do_initcalls+0x38/0x58
[ef82bfe0] [c035e2c4] kernel_init+0x30/0x90
[ef82bff0] [c000f464] kernel_thread+0x4c/0x68
[...]
And again, ucc_geth_stop() assumes that ugeth->phydev is there, while
it isn't.
This patch fixes all three oopses simply by rearranging some code:
- In ucc_geth_open(): move init_phy() call to the beginning, so
that we only call ucc_geth_stop() with a PHY attached;
- Move phy_disconnect() call from ucc_geth_close() to
ucc_geth_stop(), so that we'll always disconnect the PHY.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 1f61e42..9eef04b 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -2009,6 +2009,9 @@ static void ucc_geth_stop(struct ucc_geth_private *ugeth)
/* Disable Rx and Tx */
clrbits32(&ug_regs->maccfg1, MACCFG1_ENABLE_RX | MACCFG1_ENABLE_TX);
+ phy_disconnect(ugeth->phydev);
+ ugeth->phydev = NULL;
+
ucc_geth_memclean(ugeth);
}
@@ -3345,6 +3348,14 @@ static int ucc_geth_open(struct net_device *dev)
return -EINVAL;
}
+ err = init_phy(dev);
+ if (err) {
+ if (netif_msg_ifup(ugeth))
+ ugeth_err("%s: Cannot initialize PHY, aborting.",
+ dev->name);
+ return err;
+ }
+
err = ucc_struct_init(ugeth);
if (err) {
if (netif_msg_ifup(ugeth))
@@ -3381,13 +3392,6 @@ static int ucc_geth_open(struct net_device *dev)
&ugeth->ug_regs->macstnaddr1,
&ugeth->ug_regs->macstnaddr2);
- err = init_phy(dev);
- if (err) {
- if (netif_msg_ifup(ugeth))
- ugeth_err("%s: Cannot initialize PHY, aborting.", dev->name);
- goto out_err;
- }
-
phy_start(ugeth->phydev);
err = ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
@@ -3430,9 +3434,6 @@ static int ucc_geth_close(struct net_device *dev)
free_irq(ugeth->ug_info->uf_info.irq, ugeth->dev);
- phy_disconnect(ugeth->phydev);
- ugeth->phydev = NULL;
-
netif_stop_queue(dev);
return 0;
--
1.5.6.5
^ permalink raw reply related
* RE: sata device failed to IDENTIFY...
From: Tirumala Reddy Marri @ 2009-03-27 16:21 UTC (permalink / raw)
To: rizwan ahmad, linuxppc-dev
In-Reply-To: <22722574.post@talk.nabble.com>
What PCI Sata card is this ? Is you device tree entrees are correct for
PCI ? Especially the interrupt numbers etc ? Also check your ioremap()
function in the SATA driver. Make sure that data type of physical
address in the driver using is "unsigned long long" instead of "unsigned
long"
-----Original Message-----
From: linuxppc-dev-bounces+tmarri=3Damcc.com@ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=3Damcc.com@ozlabs.org] On Behalf Of
rizwan ahmad
Sent: Thursday, March 26, 2009 6:57 AM
To: linuxppc-dev@ozlabs.org
Subject: Re: sata device failed to IDENTIFY...
output of lspci -vv
-bash-3.2# lspci -vv
00:0c.0 Class 0c03: Unknown device 1106:3038 (rev 62)
Subsystem: Unknown device 1106:3038
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr-
Ste- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dmedium
>TAbort-
<TAbor- Latency: 128
Interrupt: pin A routed to IRQ 25
Region 4: I/O ports at ffe0 [size=3D32]
Capabilities: [80] Power Management version 2
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=3D375mA
PME(D0+,D1+,D2+,D3) Status: D0 PME-Enable- DSel=3D0
DScale=3D0
PME-
00:0c.1 Class 0c03: Unknown device 1106:3038 (rev 62)
Subsystem: Unknown device 1106:3038
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr-
Ste- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dmedium
>TAbort-
<TAbor- Latency: 128
Interrupt: pin B routed to IRQ 25
Region 4: I/O ports at ffc0 [size=3D32]
Capabilities: [80] Power Management version 2
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=3D375mA
PME(D0+,D1+,D2+,D3) Status: D0 PME-Enable- DSel=3D0
DScale=3D0
PME-
00:0c.2 Class 0c03: Unknown device 1106:3104 (rev 65) (prog-if 20)
Subsystem: Unknown device 1106:3104
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr-
Ste- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dmedium
>TAbort-
<TAbor- Latency: 128
Interrupt: pin C routed to IRQ 25
Region 0: Memory at afffff00 (32-bit, non-prefetchable)
[size=3D256]
Capabilities: [80] Power Management version 2
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=3D375mA
PME(D0+,D1+,D2+,D3) Status: D0 PME-Enable- DSel=3D0
DScale=3D0
PME-
00:0c.3 Class 0104: Unknown device 1106:3249 (rev 50)
Subsystem: Unknown device 1106:3249
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr-
Ste- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=3Dmedium
>TAbort-
<TAbor- Latency: 128
Interrupt: pin A routed to IRQ 25
Region 0: I/O ports at ffb0 [size=3D16]
Region 1: I/O ports at ffa0 [size=3D16]
Region 2: I/O ports at ff90 [size=3D16]
Region 3: I/O ports at ff80 [size=3D16]
Region 4: I/O ports at ff60 [size=3D32]
Region 5: I/O ports at fe00 [size=3D256]
Expansion ROM at <ignored> [disabled]
Capabilities: [e0] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=3D0mA
PME(D0-,D1-,D2-,D3ho) Status: D0 PME-Enable- DSel=3D0
DScale=3D0
PME-
-bash-3.2#
--=20
View this message in context:
http://www.nabble.com/sata-device-failed-to-IDENTIFY...-tp22655709p22722
574.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: issue at the beginning of kernel booting
From: Scott Wood @ 2009-03-27 16:15 UTC (permalink / raw)
To: chmhou; +Cc: linuxppc-dev
In-Reply-To: <20090327083628.4C3E93EE530@mail2-203.sinamail.sina.com.cn>
On Fri, Mar 27, 2009 at 04:36:28PM +0800, chmhou wrote:
> there is something confused me, which is my kernel code is halt because of something wrong i have no hint about it.
> my bootloader is u-boot, version is 1.1.4, kernel version is 2.6.14.1, processor is MPC8247.
That is very old code; you're more likely to get help when running
something up-to-date.
> u-boot have run on the board already. bootloader booting information as follow:
> U-Boot 1.1.4 (Mar 22 2009 - 20:30:57)
> MPC8272 Reset Status: External Soft, External Hard
> MPC8272 Clock Configuration
> - Bus-to-Core Mult 4x, VCO Div 2, 60x Bus Freq 25-75 , Core Freq 100-300
> - dfbrg 1, corecnf 0x1a, busdf 3, cpmdf 1, plldf 0, pllmf 3
> - vco_out 400000000, scc_clk 100000000, brg_clk 25000000
> - cpu_clk 400000000, cpm_clk 200000000, bus_clk 100000000
This alternative was listed as text/plain.   is not plaintext.
Please fix your mailer (and better yet, don't send HTML at all).
> by LEDs i am sure that it surely performs in kernel. then by sequence
> early_init, mmu_off, clear_bats and flush_tlbs. at call_setup_cpu, the
> performing is halt, i get "bl call_setup_cpu" will invoke
> setup_common_caches actually. right here, enable caches for 603e by
> HID0, get HID0 register and setting DCE and ICE bit put into HID0
> register, the performing halt.
I doubt that it is actually halting here; more likely, you're just
hitting a spot in the boot sequence where it's not safe to access your
LEDs or other I/O, because you have the MMU off and the cache enabled.
You'll typically be much better off leaving the very early code alone
(it's almost never the culprit), except to ensure that you have a BAT
mapping (cache-inhibited and guarded) for the I/O you want to use for
debugging. Then, only touch that I/O device *after* the MMU is on and
your mapping is in place.
-Scott
^ permalink raw reply
* Re: DTC sources move
From: Grant Likely @ 2009-03-27 15:57 UTC (permalink / raw)
To: monstr; +Cc: David Gibson, linuxppc-dev
In-Reply-To: <49CCDFBD.6020602@monstr.eu>
On Fri, Mar 27, 2009 at 8:16 AM, Michal Simek <monstr@monstr.eu> wrote:
> Hi Ben,
>
> could you please move DTC to any generic location. I would like to use it for Microblaze cpu too.
Michal,
David Gibson is the one responsible for the in-tree dtc source, not
Ben, and you should prepare the patches yourself to move dtc.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* DTC sources move
From: Michal Simek @ 2009-03-27 14:16 UTC (permalink / raw)
To: benh, Grant Likely, linuxppc-dev
Hi Ben,
could you please move DTC to any generic location. I would like to use it for Microblaze cpu too.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
^ permalink raw reply
* Re: linux-next: tracing/powerpc tree build failure
From: Stephen Rothwell @ 2009-03-27 14:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Benjamin, linuxppc-dev, linux-next, Paul Mackerras,
H. Peter Anvin, Thomas Gleixner, Ingo Molnar
In-Reply-To: <1238158131.22033.10.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 593 bytes --]
Hi Steve,
On Fri, 27 Mar 2009 08:48:51 -0400 Steven Rostedt <srostedt@redhat.com> wrote:
>
> I knew this was going to break, but I needed to wait till powerpc merged
> with tip before I could make the change.
Yeah, a bit of a pain but we can live with it for a while.
> This is exactly what I would have done.
Thanks for the confirmation. I guess I will keep applying this until one
tree or the other hits mainline at which point it can be fixed in the
other tree.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] ucc_geth: Rework the TX logic.
From: Scott Wood @ 2009-03-27 13:26 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev, Li Yang, pku.leo, netdev
In-Reply-To: <OFDCE90357.0EDC5DC1-ONC1257586.003759A3-C1257586.00390AD3@transmode.se>
Joakim Tjernlund wrote:
> This is a bit better than the current type casts. Moving over to qe_bd
> accesses is a bigger cleanup that will have to be a seperate patch.
> I am not sure it is an all win as you probably loose the ability
> to read both status and len in one access.
Not if you define the struct to have one status_len field (or a union of
that with separate fields), as gianfar does.
-Scott
^ permalink raw reply
* Re: Q: define i2c nodes in device tree?
From: Albrecht Dreß @ 2009-03-27 13:05 UTC (permalink / raw)
To: linuxppc-dev
Hi Timur:
> You need to have a "gpio I2C" device driver that registers the same name =
as what's in the DTS, like this:
Thanks, that did the trick; a proper naming is apparently
pca8575:gpio@20 {
compatible =3D "nxp,pca8575";
device_type =3D "gpio";
reg =3D <0x20 1>;
};
Now the driver is called, but complains that no platform data is present. =
Still have to figure out how to pass that... :-/
An other confusing thing: although the probe function fails with error -EIN=
VAL, the device gets registered:
<7>[ 0.801814] i2c 1-0020: uevent
<7>[ 0.802213] pcf857x 1-0020: probe
<7>[ 0.802236] pcf857x 1-0020: no platform data
<4>[ 0.802265] pcf857x: probe of 1-0020 failed with error -22
<7>[ 0.807980] i2c-adapter i2c-1: client [pca8575] registered with bus i=
d 1-0020
# ls -al /sys/devices/f0000000.soc5200/f0003d40.i2c/i2c-adapter/i2c-1/
drwxr-xr-x 6 0 0 0 Jan 1 00:00 .
drwxr-xr-x 3 0 0 0 Jan 1 00:00 ..
drwxr-xr-x 2 0 0 0 Jan 1 00:00 1-0020
drwxr-xr-x 2 0 0 0 Jan 1 00:00 1-0021
drwxr-xr-x 2 0 0 0 Jan 1 00:00 1-0022
[...]
Is this the intended behaviour?
Thanks, Albrecht.
Zerrei=DFen Sie die Netze der Phisher, Hacker und Betr=FCger!
Ihre Internet-Sicherheits-Seiten auf Arcor.de bieten alle Infos und Hilfsmi=
ttel, die Sie zum sicheren Surfen brauchen! Play it safe!
http://www.arcor.de/footer-sicherheit/
^ permalink raw reply
* Re: linux-next: tracing/powerpc tree build failure
From: Steven Rostedt @ 2009-03-27 12:48 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linuxppc-dev, linux-next, Paul Mackerras, H. Peter Anvin,
Thomas Gleixner, Ingo Molnar
In-Reply-To: <20090327230834.93d0221d.sfr@canb.auug.org.au>
On Fri, 2009-03-27 at 23:08 +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next build (powerpc allyesconfig) failed like this:
>
> arch/powerpc/kernel/ftrace.c: In function 'prepare_ftrace_return':
> arch/powerpc/kernel/ftrace.c:612: warning: passing argument 3 of 'ftrace_push_return_trace' makes pointer from integer without a cast
> arch/powerpc/kernel/ftrace.c:612: error: too many arguments to function 'ftrace_push_return_trace'
>
> Caused by commit 5d1a03dc541dc6672e60e57249ed22f40654ca47
> ("function-graph: moved the timestamp from arch to generic code") from
> the tracing tree which (removed an argument from
> ftrace_push_return_trace()) interacting with commit
> 6794c78243bfda020ab184d6d578944f8e90d26c ("powerpc64: port of the
> function graph tracer") from the powerpc tree.
>
> I added the following patch and can carry it as necessary.
Thanks Stephen!!!
I knew this was going to break, but I needed to wait till powerpc merged
with tip before I could make the change.
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 27 Mar 2009 22:47:58 +1100
> Subject: [PATCH] tracing: fixup for ftrace_push_return_trace API change
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/ftrace.c | 6 +-----
> 1 files changed, 1 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index 5b5d16b..5455943 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -557,7 +557,6 @@ extern void mod_return_to_handler(void);
> void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
> {
> unsigned long old;
> - unsigned long long calltime;
> int faulted;
> struct ftrace_graph_ent trace;
> unsigned long return_hooker = (unsigned long)&return_to_handler;
> @@ -606,10 +605,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
> return;
> }
>
> - calltime = cpu_clock(raw_smp_processor_id());
> -
> - if (ftrace_push_return_trace(old, calltime,
> - self_addr, &trace.depth) == -EBUSY) {
> + if (ftrace_push_return_trace(old, self_addr, &trace.depth) == -EBUSY) {
This is exactly what I would have done.
Acked-by: Steven Rostedt <srostedt@redhat.com>
-- Steve
> *parent = old;
> return;
> }
^ permalink raw reply
* linux-next: tracing/powerpc tree build failure
From: Stephen Rothwell @ 2009-03-27 12:08 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
Cc: Steven Rostedt, linux-next, Paul Mackerras, linuxppc-dev
Hi all,
Today's linux-next build (powerpc allyesconfig) failed like this:
arch/powerpc/kernel/ftrace.c: In function 'prepare_ftrace_return':
arch/powerpc/kernel/ftrace.c:612: warning: passing argument 3 of 'ftrace_push_return_trace' makes pointer from integer without a cast
arch/powerpc/kernel/ftrace.c:612: error: too many arguments to function 'ftrace_push_return_trace'
Caused by commit 5d1a03dc541dc6672e60e57249ed22f40654ca47
("function-graph: moved the timestamp from arch to generic code") from
the tracing tree which (removed an argument from
ftrace_push_return_trace()) interacting with commit
6794c78243bfda020ab184d6d578944f8e90d26c ("powerpc64: port of the
function graph tracer") from the powerpc tree.
I added the following patch and can carry it as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 27 Mar 2009 22:47:58 +1100
Subject: [PATCH] tracing: fixup for ftrace_push_return_trace API change
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/kernel/ftrace.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 5b5d16b..5455943 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -557,7 +557,6 @@ extern void mod_return_to_handler(void);
void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
{
unsigned long old;
- unsigned long long calltime;
int faulted;
struct ftrace_graph_ent trace;
unsigned long return_hooker = (unsigned long)&return_to_handler;
@@ -606,10 +605,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
return;
}
- calltime = cpu_clock(raw_smp_processor_id());
-
- if (ftrace_push_return_trace(old, calltime,
- self_addr, &trace.depth) == -EBUSY) {
+ if (ftrace_push_return_trace(old, self_addr, &trace.depth) == -EBUSY) {
*parent = old;
return;
}
--
1.6.2.1
^ permalink raw reply related
* CONFIG_DEBUG_PAGEALLOC generates WARN_ON() in ppc32 hash
From: Kumar Gala @ 2009-03-27 11:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: LinuxPPC Mailing List
Ben,
If we have CONFIG_DEBUG_PAGEALLOC enabled we end up hitting the
WARN_ON() in set_pte_at() when we boot. This appears because we've
already setup pte's via mapin_ram() after which we end up calling
kernel_map_pages().
So I'm not sure if __change_page_attr() should be doing something
different or if the WARN_ON() should be conditional on !
CONFIG_DEBUG_PAGEALLOC
- k
^ permalink raw reply
* Re: [PATCH 1/2] ucc_geth: Move freeing of TX packets to NAPI context.
From: Joakim Tjernlund @ 2009-03-27 11:52 UTC (permalink / raw)
To: Li Yang; +Cc: linuxppc-dev, pku.leo, netdev
In-Reply-To: <2a27d3730903270350m52fcb837ge78c0b87cc3887d5@mail.gmail.com>
pku.leo@gmail.com wrote on 27/03/2009 11:50:09:
>
> On Thu, Mar 26, 2009 at 8:54 PM, Joakim Tjernlund
> <Joakim.Tjernlund@transmode.se> wrote:
> > Also set NAPI weight to 64 as this is a common value.
> > This will make the system alot more responsive while
> > ping flooding the ucc_geth ethernet interaface.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> > /* Errors and other events */
> > if (ucce & UCCE_OTHER) {
> > if (ucce & UCC_GETH_UCCE_BSY)
> > @@ -3733,7 +3725,7 @@ static int ucc_geth_probe(struct of_device*
ofdev, const struct of_device_id *ma
> > dev->netdev_ops = &ucc_geth_netdev_ops;
> > dev->watchdog_timeo = TX_TIMEOUT;
> > INIT_WORK(&ugeth->timeout_work, ucc_geth_timeout_work);
> > - netif_napi_add(dev, &ugeth->napi, ucc_geth_poll,
UCC_GETH_DEV_WEIGHT);
> > + netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, 64);
>
> It doesn't make sense to have larger napi budget than the size of RX
> BD ring. You can't have more BDs than RX_BD_RING_LEN in backlog for
> napi_poll to process. Increase the RX_BD_RING_LEN if you want to
> increase UCC_GETH_DEV_WEIGHT. However please also provide the
> performance comparison for this kind of change. Thanks
Bring it up with David Miller. After my initial attempt to just increase
weight somewhat, he requested that I hardcoded it to 64. Just read the
whole thread.
If I don't increase weight somewhat, ping -f -l 3 almost halts the board.
Logging
in takes forever. These are my "performance numbers".
weight theory:
Before the drivers gets to the end of a full BD ring, new pkgs arrives so
that
even if the DB ring is only 16, the driver wants to process 17 or more
pkgs.
^ permalink raw reply
* Re: [PATCH v3] powerpc: clean up ssi.txt, add definition for fsl, ssi-asynchronous
From: Kumar Gala @ 2009-03-27 11:45 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <1236817324-24612-1-git-send-email-timur@freescale.com>
On Mar 11, 2009, at 7:22 PM, Timur Tabi wrote:
> Add the definition of the fsl,ssi-asynchronous property to ssi.txt
> (documentation
> of the device tree bindings for the Freescale SSI device).
>
> Also tidy up the layout of ssi.txt.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> v3: rebased
>
> v2: fixed typo, improved wording.
>
> Documentation/powerpc/dts-bindings/fsl/ssi.txt | 68 +++++++++++++
> +----------
> 1 files changed, 39 insertions(+), 29 deletions(-)
applied to next
- k
^ permalink raw reply
* Re: [PATCH v2] powerpc/85xx: Add support for the "socrates" board (MPC8544).
From: Kumar Gala @ 2009-03-27 11:45 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linuxppc-dev
In-Reply-To: <49C64413.605@grandegger.com>
On Mar 22, 2009, at 8:58 AM, Wolfgang Grandegger wrote:
> Supported are Ethernet, serial console, I2C, I2C-based RTC and
> temperature sensors, NOR and NAND flash, PCI, USB, CAN and Lime
> display controller.
>
> The multiplexing of FPGA interrupts onto PowerPC interrupt lines is
> supported through our own fpga_pic interrupt controller driver.
>
> For example the SJA1000 controller is level low sensitive connected to
> fpga_pic line 2 and is routed to the second (of three) irq lines to
> the CPU:
>
> can@3,100 {
> compatible = "philips,sja1000";
> reg = <3 0x100 0x80>;
> interrupts = <2 2>;
> interrupts = <2 8 1>; // number, type, routing
> interrupt-parent = <&fpga_pic>;
> };
>
> Signed-off-by: Sergei Poselenov <sposelenov@emcraft.com>
> Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Signed-off-by: Dmitry Rakhchev <rda@emcraft.com>
> ---
> arch/powerpc/boot/dts/socrates.dts | 338 +++++
> arch/powerpc/configs/85xx/socrates_defconfig | 1410 ++++++++++++++
> ++++++++++
> arch/powerpc/platforms/85xx/Kconfig | 6
> arch/powerpc/platforms/85xx/Makefile | 1
> arch/powerpc/platforms/85xx/socrates.c | 133 ++
> arch/powerpc/platforms/85xx/socrates_fpga_pic.c | 327 +++++
> arch/powerpc/platforms/85xx/socrates_fpga_pic.h | 16
> 7 files changed, 2231 insertions(+)
> create mode 100644 arch/powerpc/boot/dts/socrates.dts
> create mode 100644 arch/powerpc/configs/85xx/socrates_defconfig
> create mode 100644 arch/powerpc/platforms/85xx/socrates.c
> create mode 100644 arch/powerpc/platforms/85xx/socrates_fpga_pic.c
> create mode 100644 arch/powerpc/platforms/85xx/socrates_fpga_pic.h
> create mode 100644 drivers/mtd/nand/socrates_nand.c
applied to next
- k
^ permalink raw reply
* Re: [PATCH] ucc_geth: Rework the TX logic.
From: Joakim Tjernlund @ 2009-03-27 11:39 UTC (permalink / raw)
To: Li Yang; +Cc: linuxppc-dev, pku.leo, netdev
In-Reply-To: <2a27d3730903270339l44ccb55fo9079402104afc458@mail.gmail.com>
pku.leo@gmail.com wrote on 27/03/2009 11:39:07:
> On Fri, Mar 27, 2009 at 6:23 PM, Joakim Tjernlund
> >> > - /* We freed a buffer, so now we can restart
> > transmission */
> >> > - if (netif_queue_stopped(dev))
> >> > - netif_wake_queue(dev);
> >> > + dev_kfree_skb(ugeth->tx_skbuff[txQ][tx_ind]);
> >> > + ugeth->tx_skbuff[txQ][tx_ind] = NULL;
> >> > + out_be32(bd2buf(bd), 0); /* Mark it free */
> >>
> >> Now I see why you depend on the buffer to judge if the BD ring is
> >> full. If you want to do this, make sure it's well documented in
code,
> >> or others will be confused. And as Anton already commented, in/out
> >> access is slow. I think the original way can be fixed if you think
> >> it's broken, and will have better performance.
> >
> > The code could use a better comment, but I really think this method
> > is superior and more robust. The problem is that in/out functions is
> > much slower than it has to be for QE IO memory.
> >
> > The original way is broken and I tired to fix it but it always broke
> > as soon one applied some load.
>
> The only difference I see between your method and the original code is
> that you update the cleanup progress on every BD. The original code
> updates progress only after all sent BDs are processed. You can try
> move ugeth->confBd[txQ] = bd; into the loop then it will be the same
> as your proposed code.
Already tried that+lots of other combinatios. Doesn't work
^ permalink raw reply
* Re: [PATCH 1/2] ucc_geth: Move freeing of TX packets to NAPI context.
From: Li Yang @ 2009-03-27 10:50 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev, netdev
In-Reply-To: <1238072077-27044-1-git-send-email-Joakim.Tjernlund@transmode.se>
On Thu, Mar 26, 2009 at 8:54 PM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> Also set NAPI weight to 64 as this is a common value.
> This will make the system alot more responsive while
> ping flooding the ucc_geth ethernet interaface.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> =C2=A0drivers/net/ucc_geth.c | =C2=A0 32 ++++++++++++--------------------
> =C2=A0drivers/net/ucc_geth.h | =C2=A0 =C2=A01 -
> =C2=A02 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 097aed8..7fc91aa 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -3214,7 +3214,7 @@ static int ucc_geth_tx(struct net_device *dev, u8 t=
xQ)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->stats.tx_pack=
ets++;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Free the sk buf=
fer associated with this TxBD */
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb_irq(uget=
h->
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb(ugeth->
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0tx_skbuff[txQ][ugeth->skb_dirt=
ytx[txQ]]);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth->tx_skbuff[t=
xQ][ugeth->skb_dirtytx[txQ]] =3D NULL;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth->skb_dirtytx=
[txQ] =3D
> @@ -3248,9 +3248,16 @@ static int ucc_geth_poll(struct napi_struct *napi,=
int budget)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0for (i =3D 0; i < ug_info->numQueuesRx; i++)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0howmany +=3D ucc_g=
eth_rx(ugeth, i, budget - howmany);
>
> + =C2=A0 =C2=A0 =C2=A0 /* Tx event processing */
> + =C2=A0 =C2=A0 =C2=A0 spin_lock(&ugeth->lock);
> + =C2=A0 =C2=A0 =C2=A0 for (i =3D 0; i < ug_info->numQueuesTx; i++) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ucc_geth_tx(ugeth->dev=
, i);
> + =C2=A0 =C2=A0 =C2=A0 }
> + =C2=A0 =C2=A0 =C2=A0 spin_unlock(&ugeth->lock);
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (howmany < budget) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0netif_rx_complete(=
napi);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 setbits32(ugeth->uccf-=
>p_uccm, UCCE_RX_EVENTS);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 setbits32(ugeth->uccf-=
>p_uccm, UCCE_RX_EVENTS | UCCE_TX_EVENTS);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return howmany;
> @@ -3264,8 +3271,6 @@ static irqreturn_t ucc_geth_irq_handler(int irq, vo=
id *info)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct ucc_geth_info *ug_info;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0register u32 ucce;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0register u32 uccm;
> - =C2=A0 =C2=A0 =C2=A0 register u32 tx_mask;
> - =C2=A0 =C2=A0 =C2=A0 u8 i;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth_vdbg("%s: IN", __func__);
>
> @@ -3279,27 +3284,14 @@ static irqreturn_t ucc_geth_irq_handler(int irq, =
void *info)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0out_be32(uccf->p_ucce, ucce);
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* check for receive events that require proce=
ssing */
> - =C2=A0 =C2=A0 =C2=A0 if (ucce & UCCE_RX_EVENTS) {
> + =C2=A0 =C2=A0 =C2=A0 if (ucce & (UCCE_RX_EVENTS | UCCE_TX_EVENTS)) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (netif_rx_sched=
ule_prep(&ugeth->napi)) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 uccm &=3D ~UCCE_RX_EVENTS;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 uccm &=3D ~(UCCE_RX_EVENTS | UCCE_TX_EVENTS);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0out_be32(uccf->p_uccm, uccm);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0__netif_rx_schedule(&ugeth->napi);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> - =C2=A0 =C2=A0 =C2=A0 /* Tx event processing */
> - =C2=A0 =C2=A0 =C2=A0 if (ucce & UCCE_TX_EVENTS) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 spin_lock(&ugeth->lock=
);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tx_mask =3D UCC_GETH_U=
CCE_TXB0;
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (i =3D 0; i < ug_i=
nfo->numQueuesTx; i++) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 if (ucce & tx_mask)
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ucc_geth_tx(dev, i);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 ucce &=3D ~tx_mask;
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 tx_mask <<=3D 1;
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 spin_unlock(&ugeth->lo=
ck);
> - =C2=A0 =C2=A0 =C2=A0 }
> -
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Errors and other events */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ucce & UCCE_OTHER) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ucce & UCC_GET=
H_UCCE_BSY)
> @@ -3733,7 +3725,7 @@ static int ucc_geth_probe(struct of_device* ofdev, =
const struct of_device_id *ma
> =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->netdev_ops =3D &ucc_geth_netdev_ops;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->watchdog_timeo =3D TX_TIMEOUT;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0INIT_WORK(&ugeth->timeout_work, ucc_geth_timeo=
ut_work);
> - =C2=A0 =C2=A0 =C2=A0 netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, U=
CC_GETH_DEV_WEIGHT);
> + =C2=A0 =C2=A0 =C2=A0 netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, 6=
4);
It doesn't make sense to have larger napi budget than the size of RX
BD ring. You can't have more BDs than RX_BD_RING_LEN in backlog for
napi_poll to process. Increase the RX_BD_RING_LEN if you want to
increase UCC_GETH_DEV_WEIGHT. However please also provide the
performance comparison for this kind of change. Thanks
- Leo
^ permalink raw reply
* Re: [PATCH] ucc_geth: Rework the TX logic.
From: Li Yang @ 2009-03-27 10:39 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev, netdev
In-Reply-To: <OFDCE90357.0EDC5DC1-ONC1257586.003759A3-C1257586.00390AD3@transmode.se>
On Fri, Mar 27, 2009 at 6:23 PM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> pku.leo@gmail.com wrote on 27/03/2009 10:45:12:
>> On Fri, Mar 27, 2009 at 1:44 AM, Joakim Tjernlund
>> <Joakim.Tjernlund@transmode.se> wrote:
>> > The line:
>> > =C2=A0if ((bd =3D=3D ugeth->txBd[txQ]) && (netif_queue_stopped(dev) =
=3D=3D 0))
>> > =C2=A0 =C2=A0 =C2=A0 break;
>> > in ucc_geth_tx() didn not make sense to me. Rework & cleanup
>> > this logic to something understandable.
>> > ---
>> >
>> > Reworked the patch according to Antons comments.
>> >
>> > =C2=A0drivers/net/ucc_geth.c | =C2=A0 66
> +++++++++++++++++++++++++++--------------------
>> > =C2=A01 files changed, 38 insertions(+), 28 deletions(-)
>> >
>> > diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
>> > index 7fc91aa..465de3a 100644
>> > --- a/drivers/net/ucc_geth.c
>> > +++ b/drivers/net/ucc_geth.c
>> > @@ -161,6 +161,17 @@ static struct ucc_geth_info ugeth_primary_info =
=3D
> {
>> >
>> > =C2=A0static struct ucc_geth_info ugeth_info[8];
>> >
>> > +
>> > +static inline u32 __iomem *bd2buf(u8 __iomem *bd)
>> > +{
>> > + =C2=A0 =C2=A0 =C2=A0 return (u32 __iomem *)(bd+4);
>> > +}
>> > +
>> > +static inline u32 __iomem *bd2status(u8 __iomem *bd)
>> > +{
>> > + =C2=A0 =C2=A0 =C2=A0 return (u32 __iomem *)bd;
>> > +}
>>
>>
>> When the driver was reviewed for upstream merge, I was told to remove
>> this kind of thing in favor of direct struct qe_bd access. =C2=A0So do w=
e
>> really have a guideline on this? =C2=A0Or it's just a matter of personal
>> preference?
>
> This is a bit better than the current type casts. Moving over to qe_bd
> accesses is a bigger cleanup that will have to be a seperate patch.
> I am not sure it is an all win as you probably loose the ability
> to read both status and len in one access.
>
>>
>> > +
>> > =C2=A0#ifdef DEBUG
>> > =C2=A0static void mem_disp(u8 *addr, int size)
>> > =C2=A0{
>> > @@ -3048,6 +3059,7 @@ static int ucc_geth_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 __iomem *bd; =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* BD pointer */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 bd_status;
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 txQ =3D 0;
>> > + =C2=A0 =C2=A0 =C2=A0 int tx_ind;
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth_vdbg("%s: IN", __func__);
>> >
>> > @@ -3057,21 +3069,19 @@ static int ucc_geth_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Start from the next BD that should be fi=
lled */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0bd =3D ugeth->txBd[txQ];
>> > - =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32((u32 __iomem *)bd);
>> > + =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(bd2status(bd));
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Save the skb pointer so we can free it l=
ater */
>> > - =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] =
=3D skb;
>> > + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D ugeth->skb_curtx[txQ];
>> > + =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][tx_ind] =3D skb;
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Update the current skb pointer (wrapping=
if this was the
> last) */
>> > - =C2=A0 =C2=A0 =C2=A0 ugeth->skb_curtx[txQ] =3D
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (ugeth->skb_curtx[txQ] +
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A01) & TX_RING_MOD_MASK(ugeth=
->ug_info->bdRingLenTx[txQ]);
>> > + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D (tx_ind + 1) &
> TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
>> > + =C2=A0 =C2=A0 =C2=A0 ugeth->skb_curtx[txQ] =3D tx_ind;
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* set up the buffer descriptor */
>> > - =C2=A0 =C2=A0 =C2=A0 out_be32(&((struct qe_bd __iomem *)bd)->buf,
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 dma_map_single(&ugeth->dev->dev, skb->data,
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 skb->len, DMA_TO_DEVICE));
>> > -
>> > - =C2=A0 =C2=A0 =C2=A0 /* printk(KERN_DEBUG"skb->data is 0x%x\n",skb->=
data); */
>> > + =C2=A0 =C2=A0 =C2=A0 out_be32(bd2buf(bd),
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dma_map_singl=
e(&ugeth->dev->dev, skb->data,
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 skb->len, DMA_TO_DEVICE));
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0bd_status =3D (bd_status & T_W) | T_R | T_I=
| T_L | skb->len;
>> >
>> > @@ -3088,9 +3098,9 @@ static int ucc_geth_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* If the next BD still needs to be cleaned=
up, then the bds
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 are full. =C2=A0We need to tell the=
kernel to stop sending us
> stuff. */
>> > - =C2=A0 =C2=A0 =C2=A0 if (bd =3D=3D ugeth->confBd[txQ]) {
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!netif_queue_st=
opped(dev))
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 netif_stop_queue(dev);
>> > +
>> > + =C2=A0 =C2=A0 =C2=A0 if (!in_be32(bd2buf(bd))) {
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 netif_stop_queue(de=
v);
>>
>> Why does this make more sense? =C2=A0The BD with a NULL buffer means the
>> ring is full? =C2=A0It's not the normal case.
>>
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth->txBd[txQ] =3D bd;
>> > @@ -3198,41 +3208,41 @@ static int ucc_geth_tx(struct net_device *dev,
> u8 txQ)
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0struct ucc_geth_private *ugeth =3D netdev_p=
riv(dev);
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 __iomem *bd; =C2=A0 =C2=A0 =C2=A0 =C2=A0=
/* BD pointer */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 bd_status;
>> > + =C2=A0 =C2=A0 =C2=A0 int tx_ind, num_freed;
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0bd =3D ugeth->confBd[txQ];
>> > - =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32((u32 __iomem *)bd);
>> > -
>> > + =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(bd2status(bd));
>> > + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D ugeth->skb_dirtytx[txQ];
>> > + =C2=A0 =C2=A0 =C2=A0 num_freed =3D 0;
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Normal processing. */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0while ((bd_status & T_R) =3D=3D 0) {
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* BD contains =
already transmitted buffer. =C2=A0 */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Handle the t=
ransmitted buffer and release */
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* the BD to be=
used with the current frame =C2=A0*/
>> >
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((bd =3D=3D uget=
h->txBd[txQ]) &&
> (netif_queue_stopped(dev) =3D=3D 0))
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 break;
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (bd =3D=3D ugeth=
->txBd[txQ])
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 break; /* Queue is empty */
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->stats.tx_p=
ackets++;
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Free the sk =
buffer associated with this TxBD */
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb(ugeth=
->
>> > -
> tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]]);
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[tx=
Q][ugeth->skb_dirtytx[txQ]] =3D NULL;
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->skb_dirtytx[=
txQ] =3D
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (uget=
h->skb_dirtytx[txQ] +
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A01) &
> TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
>> > -
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* We freed a buffe=
r, so now we can restart
> transmission */
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (netif_queue_sto=
pped(dev))
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 netif_wake_queue(dev);
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb(ugeth=
->tx_skbuff[txQ][tx_ind]);
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[tx=
Q][tx_ind] =3D NULL;
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out_be32(bd2buf(bd)=
, 0); /* Mark it free */
>>
>> Now I see why you depend on the buffer to judge if the BD ring is
>> full. =C2=A0If you want to do this, make sure it's well documented in co=
de,
>> or others will be confused. =C2=A0And as Anton already commented, in/out
>> access is slow. =C2=A0I think the original way can be fixed if you think
>> it's broken, and will have better performance.
>
> The code could use a better comment, but I really think this method
> is superior and more robust. The problem is that in/out functions is
> much slower than it has to be for QE IO memory.
>
> The original way is broken and I tired to fix it but it always broke
> as soon one applied some load.
The only difference I see between your method and the original code is
that you update the cleanup progress on every BD. The original code
updates progress only after all sent BDs are processed. You can try
move ugeth->confBd[txQ] =3D bd; into the loop then it will be the same
as your proposed code.
- Leo
^ permalink raw reply
* Re: [PATCH] ucc_geth: Rework the TX logic.
From: Joakim Tjernlund @ 2009-03-27 10:23 UTC (permalink / raw)
To: Li Yang; +Cc: linuxppc-dev, pku.leo, netdev
In-Reply-To: <2a27d3730903270245k6e8633eehfb5cd3fcebd36240@mail.gmail.com>
pku.leo@gmail.com wrote on 27/03/2009 10:45:12:
> On Fri, Mar 27, 2009 at 1:44 AM, Joakim Tjernlund
> <Joakim.Tjernlund@transmode.se> wrote:
> > The line:
> > if ((bd == ugeth->txBd[txQ]) && (netif_queue_stopped(dev) == 0))
> > break;
> > in ucc_geth_tx() didn not make sense to me. Rework & cleanup
> > this logic to something understandable.
> > ---
> >
> > Reworked the patch according to Antons comments.
> >
> > drivers/net/ucc_geth.c | 66
+++++++++++++++++++++++++++--------------------
> > 1 files changed, 38 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> > index 7fc91aa..465de3a 100644
> > --- a/drivers/net/ucc_geth.c
> > +++ b/drivers/net/ucc_geth.c
> > @@ -161,6 +161,17 @@ static struct ucc_geth_info ugeth_primary_info =
{
> >
> > static struct ucc_geth_info ugeth_info[8];
> >
> > +
> > +static inline u32 __iomem *bd2buf(u8 __iomem *bd)
> > +{
> > + return (u32 __iomem *)(bd+4);
> > +}
> > +
> > +static inline u32 __iomem *bd2status(u8 __iomem *bd)
> > +{
> > + return (u32 __iomem *)bd;
> > +}
>
>
> When the driver was reviewed for upstream merge, I was told to remove
> this kind of thing in favor of direct struct qe_bd access. So do we
> really have a guideline on this? Or it's just a matter of personal
> preference?
This is a bit better than the current type casts. Moving over to qe_bd
accesses is a bigger cleanup that will have to be a seperate patch.
I am not sure it is an all win as you probably loose the ability
to read both status and len in one access.
>
> > +
> > #ifdef DEBUG
> > static void mem_disp(u8 *addr, int size)
> > {
> > @@ -3048,6 +3059,7 @@ static int ucc_geth_start_xmit(struct sk_buff
*skb, struct net_device *dev)
> > u8 __iomem *bd; /* BD pointer */
> > u32 bd_status;
> > u8 txQ = 0;
> > + int tx_ind;
> >
> > ugeth_vdbg("%s: IN", __func__);
> >
> > @@ -3057,21 +3069,19 @@ static int ucc_geth_start_xmit(struct sk_buff
*skb, struct net_device *dev)
> >
> > /* Start from the next BD that should be filled */
> > bd = ugeth->txBd[txQ];
> > - bd_status = in_be32((u32 __iomem *)bd);
> > + bd_status = in_be32(bd2status(bd));
> > /* Save the skb pointer so we can free it later */
> > - ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb;
> > + tx_ind = ugeth->skb_curtx[txQ];
> > + ugeth->tx_skbuff[txQ][tx_ind] = skb;
> >
> > /* Update the current skb pointer (wrapping if this was the
last) */
> > - ugeth->skb_curtx[txQ] =
> > - (ugeth->skb_curtx[txQ] +
> > - 1) & TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
> > + tx_ind = (tx_ind + 1) &
TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
> > + ugeth->skb_curtx[txQ] = tx_ind;
> >
> > /* set up the buffer descriptor */
> > - out_be32(&((struct qe_bd __iomem *)bd)->buf,
> > - dma_map_single(&ugeth->dev->dev, skb->data,
> > - skb->len, DMA_TO_DEVICE));
> > -
> > - /* printk(KERN_DEBUG"skb->data is 0x%x\n",skb->data); */
> > + out_be32(bd2buf(bd),
> > + dma_map_single(&ugeth->dev->dev, skb->data,
> > + skb->len, DMA_TO_DEVICE));
> >
> > bd_status = (bd_status & T_W) | T_R | T_I | T_L | skb->len;
> >
> > @@ -3088,9 +3098,9 @@ static int ucc_geth_start_xmit(struct sk_buff
*skb, struct net_device *dev)
> >
> > /* If the next BD still needs to be cleaned up, then the bds
> > are full. We need to tell the kernel to stop sending us
stuff. */
> > - if (bd == ugeth->confBd[txQ]) {
> > - if (!netif_queue_stopped(dev))
> > - netif_stop_queue(dev);
> > +
> > + if (!in_be32(bd2buf(bd))) {
> > + netif_stop_queue(dev);
>
> Why does this make more sense? The BD with a NULL buffer means the
> ring is full? It's not the normal case.
>
> > }
> >
> > ugeth->txBd[txQ] = bd;
> > @@ -3198,41 +3208,41 @@ static int ucc_geth_tx(struct net_device *dev,
u8 txQ)
> > struct ucc_geth_private *ugeth = netdev_priv(dev);
> > u8 __iomem *bd; /* BD pointer */
> > u32 bd_status;
> > + int tx_ind, num_freed;
> >
> > bd = ugeth->confBd[txQ];
> > - bd_status = in_be32((u32 __iomem *)bd);
> > -
> > + bd_status = in_be32(bd2status(bd));
> > + tx_ind = ugeth->skb_dirtytx[txQ];
> > + num_freed = 0;
> > /* Normal processing. */
> > while ((bd_status & T_R) == 0) {
> > /* BD contains already transmitted buffer. */
> > /* Handle the transmitted buffer and release */
> > /* the BD to be used with the current frame */
> >
> > - if ((bd == ugeth->txBd[txQ]) &&
(netif_queue_stopped(dev) == 0))
> > - break;
> > + if (bd == ugeth->txBd[txQ])
> > + break; /* Queue is empty */
> >
> > dev->stats.tx_packets++;
> >
> > /* Free the sk buffer associated with this TxBD */
> > - dev_kfree_skb(ugeth->
> > -
tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]]);
> > - ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]] = NULL;
> > - ugeth->skb_dirtytx[txQ] =
> > - (ugeth->skb_dirtytx[txQ] +
> > - 1) &
TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
> > -
> > - /* We freed a buffer, so now we can restart
transmission */
> > - if (netif_queue_stopped(dev))
> > - netif_wake_queue(dev);
> > + dev_kfree_skb(ugeth->tx_skbuff[txQ][tx_ind]);
> > + ugeth->tx_skbuff[txQ][tx_ind] = NULL;
> > + out_be32(bd2buf(bd), 0); /* Mark it free */
>
> Now I see why you depend on the buffer to judge if the BD ring is
> full. If you want to do this, make sure it's well documented in code,
> or others will be confused. And as Anton already commented, in/out
> access is slow. I think the original way can be fixed if you think
> it's broken, and will have better performance.
The code could use a better comment, but I really think this method
is superior and more robust. The problem is that in/out functions is
much slower than it has to be for QE IO memory.
The original way is broken and I tired to fix it but it always broke
as soon one applied some load.
Jocke
^ permalink raw reply
* Re: [PATCH] ucc_geth: Rework the TX logic.
From: Li Yang @ 2009-03-27 9:45 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev, netdev
In-Reply-To: <1238089445-28396-1-git-send-email-Joakim.Tjernlund@transmode.se>
On Fri, Mar 27, 2009 at 1:44 AM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> The line:
> =C2=A0if ((bd =3D=3D ugeth->txBd[txQ]) && (netif_queue_stopped(dev) =3D=
=3D 0))
> =C2=A0 =C2=A0 =C2=A0 break;
> in ucc_geth_tx() didn not make sense to me. Rework & cleanup
> this logic to something understandable.
> ---
>
> Reworked the patch according to Antons comments.
>
> =C2=A0drivers/net/ucc_geth.c | =C2=A0 66 +++++++++++++++++++++++++++-----=
---------------
> =C2=A01 files changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 7fc91aa..465de3a 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -161,6 +161,17 @@ static struct ucc_geth_info ugeth_primary_info =3D {
>
> =C2=A0static struct ucc_geth_info ugeth_info[8];
>
> +
> +static inline u32 __iomem *bd2buf(u8 __iomem *bd)
> +{
> + =C2=A0 =C2=A0 =C2=A0 return (u32 __iomem *)(bd+4);
> +}
> +
> +static inline u32 __iomem *bd2status(u8 __iomem *bd)
> +{
> + =C2=A0 =C2=A0 =C2=A0 return (u32 __iomem *)bd;
> +}
When the driver was reviewed for upstream merge, I was told to remove
this kind of thing in favor of direct struct qe_bd access. So do we
really have a guideline on this? Or it's just a matter of personal
preference?
> +
> =C2=A0#ifdef DEBUG
> =C2=A0static void mem_disp(u8 *addr, int size)
> =C2=A0{
> @@ -3048,6 +3059,7 @@ static int ucc_geth_start_xmit(struct sk_buff *skb,=
struct net_device *dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 __iomem *bd; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* BD pointer */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 bd_status;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 txQ =3D 0;
> + =C2=A0 =C2=A0 =C2=A0 int tx_ind;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth_vdbg("%s: IN", __func__);
>
> @@ -3057,21 +3069,19 @@ static int ucc_geth_start_xmit(struct sk_buff *sk=
b, struct net_device *dev)
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Start from the next BD that should be fille=
d */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0bd =3D ugeth->txBd[txQ];
> - =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32((u32 __iomem *)bd);
> + =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(bd2status(bd));
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Save the skb pointer so we can free it late=
r */
> - =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] =3D s=
kb;
> + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D ugeth->skb_curtx[txQ];
> + =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][tx_ind] =3D skb;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Update the current skb pointer (wrapping if=
this was the last) */
> - =C2=A0 =C2=A0 =C2=A0 ugeth->skb_curtx[txQ] =3D
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (ugeth->skb_curtx[txQ] +
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A01) & TX_RING_MOD_MASK(ugeth->u=
g_info->bdRingLenTx[txQ]);
> + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D (tx_ind + 1) & TX_RING_MOD_MASK(ugeth->=
ug_info->bdRingLenTx[txQ]);
> + =C2=A0 =C2=A0 =C2=A0 ugeth->skb_curtx[txQ] =3D tx_ind;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* set up the buffer descriptor */
> - =C2=A0 =C2=A0 =C2=A0 out_be32(&((struct qe_bd __iomem *)bd)->buf,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 d=
ma_map_single(&ugeth->dev->dev, skb->data,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 skb->len, DMA_TO_DEVICE));
> -
> - =C2=A0 =C2=A0 =C2=A0 /* printk(KERN_DEBUG"skb->data is 0x%x\n",skb->dat=
a); */
> + =C2=A0 =C2=A0 =C2=A0 out_be32(bd2buf(bd),
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dma_map_single(&=
ugeth->dev->dev, skb->data,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 skb->len, DMA_TO_DEVICE));
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0bd_status =3D (bd_status & T_W) | T_R | T_I | =
T_L | skb->len;
>
> @@ -3088,9 +3098,9 @@ static int ucc_geth_start_xmit(struct sk_buff *skb,=
struct net_device *dev)
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* If the next BD still needs to be cleaned up=
, then the bds
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 are full. =C2=A0We need to tell the ke=
rnel to stop sending us stuff. */
> - =C2=A0 =C2=A0 =C2=A0 if (bd =3D=3D ugeth->confBd[txQ]) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!netif_queue_stopp=
ed(dev))
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 netif_stop_queue(dev);
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!in_be32(bd2buf(bd))) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 netif_stop_queue(dev);
Why does this make more sense? The BD with a NULL buffer means the
ring is full? It's not the normal case.
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth->txBd[txQ] =3D bd;
> @@ -3198,41 +3208,41 @@ static int ucc_geth_tx(struct net_device *dev, u8=
txQ)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct ucc_geth_private *ugeth =3D netdev_priv=
(dev);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u8 __iomem *bd; =C2=A0 =C2=A0 =C2=A0 =C2=A0 /*=
BD pointer */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 bd_status;
> + =C2=A0 =C2=A0 =C2=A0 int tx_ind, num_freed;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0bd =3D ugeth->confBd[txQ];
> - =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32((u32 __iomem *)bd);
> -
> + =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(bd2status(bd));
> + =C2=A0 =C2=A0 =C2=A0 tx_ind =3D ugeth->skb_dirtytx[txQ];
> + =C2=A0 =C2=A0 =C2=A0 num_freed =3D 0;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Normal processing. */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0while ((bd_status & T_R) =3D=3D 0) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* BD contains alr=
eady transmitted buffer. =C2=A0 */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Handle the tran=
smitted buffer and release */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* the BD to be us=
ed with the current frame =C2=A0*/
>
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((bd =3D=3D ugeth->=
txBd[txQ]) && (netif_queue_stopped(dev) =3D=3D 0))
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 break;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (bd =3D=3D ugeth->t=
xBd[txQ])
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 break; /* Queue is empty */
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->stats.tx_pack=
ets++;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Free the sk buf=
fer associated with this TxBD */
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb(ugeth->
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tx_skbuff[txQ][ugeth->skb_dirtytx=
[txQ]]);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][=
ugeth->skb_dirtytx[txQ]] =3D NULL;
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->skb_dirtytx[txQ=
] =3D
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (ugeth->=
skb_dirtytx[txQ] +
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A01)=
& TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
> -
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* We freed a buffer, =
so now we can restart transmission */
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (netif_queue_stoppe=
d(dev))
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 netif_wake_queue(dev);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_kfree_skb(ugeth->t=
x_skbuff[txQ][tx_ind]);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ugeth->tx_skbuff[txQ][=
tx_ind] =3D NULL;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 out_be32(bd2buf(bd), 0=
); /* Mark it free */
Now I see why you depend on the buffer to judge if the BD ring is
full. If you want to do this, make sure it's well documented in code,
or others will be confused. And as Anton already commented, in/out
access is slow. I think the original way can be fixed if you think
it's broken, and will have better performance.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 num_freed++;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tx_ind =3D (tx_ind + 1=
) & TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Advance the con=
firmation BD pointer */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!(bd_status & =
T_W))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0bd +=3D sizeof(struct qe_bd);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0bd =3D ugeth->p_tx_bd_ring[txQ];
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(=
(u32 __iomem *)bd);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bd_status =3D in_be32(=
bd2status(bd));
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> =C2=A0 =C2=A0 =C2=A0 =C2=A0ugeth->confBd[txQ] =3D bd;
> + =C2=A0 =C2=A0 =C2=A0 ugeth->skb_dirtytx[txQ] =3D tx_ind;
> + =C2=A0 =C2=A0 =C2=A0 if (num_freed)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 netif_wake_queue(dev);=
/* We freed some buffers, so restart transmission */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> =C2=A0}
>
^ permalink raw reply
* Re: [PATCH 2/2] ucc_geth: Rework the TX logic.
From: Joakim Tjernlund @ 2009-03-27 9:37 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, leoli, netdev
In-Reply-To: <20090327.004237.113711845.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote on 27/03/2009 08:42:37:
>
>
> These patches don't apply to the current tree, could you please
> respin your final version against at the very least Linus's tree?
That is strange, had a look at linus tree and I don't see any
changes in his tree that touches the same area/functions.
If I get the time I will look at it a bit more but for now I am
stuck on 2.6.29
Jocke
^ permalink raw reply
* ioremap_nocache on the nor flash, reading sporadically fails
From: DI BACCO ANTONIO - technolabs @ 2009-03-27 8:33 UTC (permalink / raw)
To: linuxppc-dev
I have an MPC880 with linux-2.6.19, I use a flash sector of my nor flash
to store some parameters.=20
In order to read them I mapped it with ioremap_nocache and most of the
time the reading is working fine. Sporadically the read returns strange
values...=20
Could it be an hardware problem? Or mapping the flash is not permitted?
Consider that linux is also stored on the flash and it is working fine.
My only doubt is that on the uC bus there is also an fpga that is
activated only after linux hgas started.=20
Bye,
Antonio.
^ permalink raw reply
* issue at the beginning of kernel booting
From: chmhou @ 2009-03-27 8:36 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]
there is something confused me, which is my kernel code is halt because of something wrong i have no hint about it.
my bootloader is u-boot, version is 1.1.4, kernel version is 2.6.14.1, processor is MPC8247.
u-boot have run on the board already. bootloader booting information as follow:
U-Boot 1.1.4 (Mar 22 2009 - 20:30:57)
MPC8272 Reset Status: External Soft, External Hard
MPC8272 Clock Configuration
- Bus-to-Core Mult 4x, VCO Div 2, 60x Bus Freq 25-75 , Core Freq 100-300
- dfbrg 1, corecnf 0x1a, busdf 3, cpmdf 1, plldf 0, pllmf 3
- vco_out 400000000, scc_clk 100000000, brg_clk 25000000
- cpu_clk 400000000, cpm_clk 200000000, bus_clk 100000000
Board: Motorola MPC8272ADS
DRAM: 32 MB
FLASH: 512 kB
In: serial
Out: serial
Err: serial
Net: FCC1 ETHERNET
Hit any key to stop autoboot: 0
then i "tftp uImage" and "bootm" with no parameters, i suppose that booting kernel will be normal at beginning even if there is no any parameters.
=> tftp
Using FCC1 ETHERNET device
TFTP from server 192.168.0.99; our IP address is 192.168.0.100
Filename 'uImage'.
Load address: 0x400000
Loading: #################################################################
#################################################################
########################################################
done
Bytes transferred = 952157 (e875d hex)
=> bootm
## Booting image at 00400000 ...
Image Name: Linux-2.6.14
Image Type: PowerPC Linux Kernel Image (gzip compressed)
Data Size: 952093 Bytes = 929.8 kB
Load Address: 00000000
Entry Point: 00000000
Verifying Checksum ... OK
OK
by LEDs i am sure that it surely performs in kernel. then by sequence early_init, mmu_off, clear_bats and flush_tlbs.
at call_setup_cpu, the performing is halt, i get "bl call_setup_cpu" will invoke setup_common_caches actually. right here, enable caches for 603e by HID0, get HID0 register and setting DCE and ICE bit put into HID0 register, the performing halt.
_GLOBAL(__setup_cpu_603)
b setup_common_caches
setup_common_caches:
mfspr r11,SPRN_HID0
andi. r0,r11,HID0_DCE
ori r11,r11,HID0_ICE|HID0_DCE
ori r8,r11,HID0_ICFI
bne 1f /* don't invalidate the D-cache */
/* halt here */
ori r8,r8,HID0_DCI /* unless it wasn't enabled */
1: sync
mtspr SPRN_HID0,r8 /* enable and invalidate caches */
sync
mtspr SPRN_HID0,r11 /* enable caches */
sync
isync
blr
then after i clear HI0_DCI , performing will run till MSR[DR] is set, i clear that bit. will run again.
i have nerver changed the code because i think there is something be independent of my board device. isnt?
why the performing halt there ? or do you think there is something wrong in the front?
Cheers~
Sauce
-------------------------------------------------------------------
新浪空间——与朋友开心分享网络新生活！(http://space.sina.com.cn/ )
[-- Attachment #2: Type: text/html, Size: 3908 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/4] powerpc: NAND: FSL UPM: document new bindings
From: Wolfgang Grandegger @ 2009-03-27 8:07 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, linux-mtd, devicetree-discuss list
In-Reply-To: <fa686aa40903261622wa2a93f9i98435080c7ab6cea@mail.gmail.com>
Grant Likely wrote:
> On Thu, Mar 26, 2009 at 4:14 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Anton Vorontsov wrote:
>>> On Thu, Mar 26, 2009 at 11:02:06AM -0600, Grant Likely wrote:
>>>> In other words, this device is not register-level compatible with the
>>>> fsl,upm-nand device. Give the node a new compatible value
>>>> (tqc,tqm8548-upm-nand) and add another entry to the of_fun_match table
>>>> for the new device. Use the .data element in the match table to
>>>> supply an alternate fun_cmd_ctrl() function for this board (instead of
>>>> using a property value do decide which fun_cmd_ctrl() behaviour to
>>>> use). New boards that *do* use the same addressing scheme can claim
>>>> compatibility with tqc,tqm8548-upm-nand.
>>> I don't like this. :-/
>>>
>>> UPM is an universal thing, so there are thousands of ways we can
>>> connect NAND to the UPM. Of which only ~10 would be sane (others are
>>> insane, and nobody would do this. If they do, _then_ we'll fall back
>>> to <board>-upm-nand scheme for a particular board).
>> Yep.
>>
>>> I don't see any problem with fsl,upm-addr-line-cs-offsets. It can
>>> describe any scheme in "addr lines are cs" connection, it's a common
>>> setup for multi-chip memory, we shouldn't treat it is as something
>>> extraordinary.
>> I fully agree. I'm going to provide a patch on monday.
>
> Well, I still don't think it is the wisest choice. My position is
> that it is better to be conservative and pedantic now because it is
> easy to relax the rules from that point. If it turns out after some
> experience with "fsl,upm-addr-line-cs-offset" that the scheme has a
> serious flaw, then the impact is contained. On the other side, if it
> is confirmed and useful and correct, it is a trivial change to make it
> available to everything that claims compatibility with fsl,upm-nand.
>
> That said, I won't oppose it if you go this route. However at the
> very least, please change the nand node's compatible list to be:
>
> compatible = "tqc,tqm8548-upm-nand", "fsl,upm-nand";
>
> The custom glue logic makes it something unique, so "tqc,..." should
> be at the start of the list to describe it as such, even if the driver
> only ever uses "fsl,upm-nand".
That's a good idea in case we need it lateron. For the time being, I
prefer to make the driver as generic as possible. Currently there are
only two boards using the driver, the MPC8360RTDK and the TQM8548. and
it's unlikely that there will be much more in the future.
Wolfgang.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox