LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 52/61] cpm_uart: Issue STOP_TX command before initializing console.
From: Vitaly Bordug @ 2007-07-18  8:00 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013608.GX15238@ld0162-tx32.am.freescale.net>

On Tue, 17 Jul 2007 20:36:08 -0500
Scott Wood wrote:

> This prevents some bootloader/bootwrapper characters from being lost.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/serial/cpm_uart/cpm_uart_core.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
> b/drivers/serial/cpm_uart/cpm_uart_core.c index 805ca46..7772c87
> 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1310,6 +1310,8 @@ static int __init cpm_uart_console_setup(struct
> console *co, char *options) udbg_putc = NULL;
>  #endif
>  
> +	cpm_line_cr_cmd(pinfo, CPM_CR_STOP_TX);
> +

I am recalling exactly the contrary patch that removes stuff to get the non-console UARTs work.
Let's better revalidate this once other pieces will be in before applying.


>  	if (IS_SMC(pinfo)) {
>  		pinfo->smcp->smc_smcm &= ~(SMCM_RX | SMCM_TX);
>  		pinfo->smcp->smc_smcmr &= ~(SMCMR_REN | SMCMR_TEN);
> @@ -1331,6 +1333,7 @@ static int __init cpm_uart_console_setup(struct
> console *co, char *options) cpm_uart_init_scc(pinfo);
>  
>  	uart_set_options(port, co, baud, parity, bits, flow);
> +	cpm_line_cr_cmd(pinfo, CPM_CR_RESTART_TX);
>  
>  	return 0;
>  }


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 46/61] mpc885ads: Rework initialization.
From: Vitaly Bordug @ 2007-07-18  8:44 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013600.GR15238@ld0162-tx32.am.freescale.net>

Overall looks good, though mentioned mutually exclusive SoC devices adding some pain
and extra code having relevant parts removed but not covered (see below).

I think it makes sense to work ontop of this code to address known quirks/issues though, hence will cover this.
yet, there are little chances to play with the 8xx inside this merge window, so I basically don't object from it being merged
as is.

On Tue, 17 Jul 2007 20:36:00 -0500
Scott Wood wrote:
> -#ifdef CONFIG_SERIAL_CPM_SMC1
> -	clrbits32(bcsr_io, BCSR1_RS232EN_1);
> -	clrbits32(&cp->cp_simode, 0xe0000000 >> 17);	/* brg1
> */
> -	tmpval8 = in_8(&(cp->cp_smc[0].smc_smcm)) | (SMCM_RX |
> SMCM_TX);
> -	out_8(&(cp->cp_smc[0].smc_smcm), tmpval8);
> -	clrbits16(&cp->cp_smc[0].smc_smcmr, SMCMR_REN |
> SMCMR_TEN);	/* brg1 */ -#else
> -	setbits32(bcsr_io,BCSR1_RS232EN_1);
> -	out_be16(&cp->cp_smc[0].smc_smcmr, 0);
> -	out_8(&cp->cp_smc[0].smc_smce, 0);
> -#endif

these are not just for beauty: if corresponding not-used uart regs are not cleared, second one has a
good chances to be hosed.

> -void init_scc_ioports(struct fs_platform_info *fpi)
> -{
> -	int scc_no = fs_get_scc_index(fpi->fs_no);
> +	/* The SCC3 enet registers overlap the SMC1 registers, so
> +	 * one of the two must be removed from the device tree.
> +	 */
Unfortunately,
this approach has very little chances to work. IIRC, SCC3 eth and SMC1 do have overlapping pins,
and just removing it from the tree is not going to save the world because now we have all-in-one early condensed
pins setup.

Also, we have to be very careful with corresponding ports shutdown: say turn off smc1 if scc3 is enabled: most prolly
it was turned on by the firmware/bootwrapper, and eth won't be alive.


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 51/61] cpm_uart: Be an of_platform device when using arch/powerpc.
From: Vitaly Bordug @ 2007-07-18  8:53 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013606.GW15238@ld0162-tx32.am.freescale.net>

On Tue, 17 Jul 2007 20:36:06 -0500
Scott Wood wrote:

> The existing OF glue code was crufty and broken.  Rather than fix it,
> it has been removed, and the serial driver now talks to the device
> tree directly.
> 
> The non-CONFIG_PPC_MERGE code can do away once CPM platforms are
> dropped from arch/ppc (which will hopefully be soon).

Was is verified to boot on arch/ppc setup? 

-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: compile error if CONFIG_BLOCK not enabled related to linux/ide.h include
From: Bartlomiej Zolnierkiewicz @ 2007-07-18 10:47 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <Pine.LNX.4.64.0707172353050.29097@blarg.am.freescale.net>


Hi,

On Wednesday 18 July 2007, Kumar Gala wrote:
> M:      bzolnier@gmail.com
> L:      linux-ide@vger.kernel.org
> 
> We get the following compile error if CONFIG_BLOCK isn't enabled:
> 
>   CC      arch/powerpc/kernel/setup_32.o
> In file included from arch/powerpc/kernel/setup_32.c:14:
> include/linux/ide.h:558: error: expected specifier-qualifier-list before 'request_queue_t'
> include/linux/ide.h:696: warning: 'struct request' declared inside parameter list
> include/linux/ide.h:696: warning: its scope is only this definition or declaration, which is probably not what you want
> include/linux/ide.h:820: warning: 'struct request' declared inside parameter list
> include/linux/ide.h:853: error: field 'wrq' has incomplete type
> include/linux/ide.h:1205: error: expected ')' before '*' token
> make[1]: *** [arch/powerpc/kernel/setup_32.o] Error 1
> make: *** [arch/powerpc/kernel] Error 2

include/linux/ide.h is exclusively for IDE subsystem

> What I'm trying to figure out is if include/linux/ide.h should be wrapped
> in a #if defined(CONFIG_IDE) || defined(CONFIG_IDE_MODULE) or if there is
> some other desired way to handle this.

Plese remove <linux/ide.h> include, there doesn't seem to be any piece of
code in arch/powerpc/kernel/setup_32.c which needs it anyway.

Thanks,
Bart

^ permalink raw reply

* RE: OF devices and non OF devices
From: Kári Davíðsson @ 2007-07-18 11:13 UTC (permalink / raw)
  To: John Rigby; +Cc: linuxppc-embedded
In-Reply-To: <4b73d43f0707051020o67e671dft1d3b3816776d8eb7@mail.gmail.com>

Yes there was indeed.

Combination of my misunderstanding, device trees and board specific =
initialization.

Things are working now.

Thanks,
kd


________________________________

From: John Rigby [mailto:jcrigby@gmail.com]=20
Sent: 5. j=FAl=ED 2007 17:21
To: K=E1ri Dav=ED=F0sson
Cc: linuxppc-embedded@ozlabs.org
Subject: Re: OF devices and non OF devices


There must be something else wrong with your configuration.
On my Lite5200B fsl_i2c_probe gets called with no changes to the driver.

The kernel version is 2.6.22-rc7

The relevant part of my device tree is:=20

        i2c@3d00 {
            device_type =3D "i2c";
            compatible =3D "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
            cell-index =3D <0>;
            reg =3D <3d00 40>;=20
            interrupts =3D <2 f 0>;
            interrupt-parent =3D <&mpc5200_pic>;
            fsl5200-clocking;
        };

        i2c@3d40 {
            device_type =3D "i2c";=20
            compatible =3D "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
            cell-index =3D <1>;
            reg =3D <3d40 40>;
            interrupts =3D <2 10 0>;
            interrupt-parent =3D <&mpc5200_pic>;=20
            fsl5200-clocking;
        };

I turned on DEBUG in drivers/base/dd.c and a call to pr_debug in the =
probe routine
and here are the relevant log messages:

[   27.258245] platform: Matched Device fsl-i2c.0 with Driver fsl-i2c
[   27.258269] platform: Probing driver fsl-i2c with device fsl-i2c.0
[   27.258299] I2C: here in fsl_i2c_probe
[   27.258732] bound device 'fsl-i2c.0' to driver 'fsl-i2c'=20
[   27.258756] platform: Bound Device fsl-i2c.0 to Driver fsl-i2c
[   27.258776] platform: Matched Device fsl-i2c.1 with Driver fsl-i2c
[   27.258789] platform: Probing driver fsl-i2c with device fsl-i2c.1
[   27.258821] I2C: here in fsl_i2c_probe
[   27.259269] bound device 'fsl-i2c.1' to driver 'fsl-i2c'
[   27.259293] platform: Bound Device fsl-i2c.1 to Driver fsl-i2c

John


On 7/5/07, John Rigby <jcrigby@gmail.com> wrote:=20

	kd,
=09
	Ok, obviously It doesn't work the way I thought.  Hopefully someone who =
does
	understand this will comment.
=09
	John=20
=09
=09
=09
	On 7/4/07, K=E1ri Dav=ED=F0sson <kari.davidsson@marel.is> wrote:=20

		John, thank you for your answare.
	=09
		Enabling CONFIG_FSL_SOC only enabled the execution of the init =
function (fsl_i2c_init())
		of the fsl-i2c driver (i2c-mpc.c). The .probe function of the driver =
was never called
		until I converted the driver to the OF model and added the =
.match_table to the driver structure.
	=09
		Then I get the .probe function (fsl_i2c_probe()) called and the i2c =
bus set up.
	=09
		Similar thing happens for the i2c device PCF8563 i.e., the init =
functin of the driver (pcf8563_init())
		is called the driver is registered with the kernel, but the .probe =
(pcf8563_probe()) is never called.
	=09
		The driver pcf8563 has _NO_ exported structures or functions so I =
basically have no handle on it=20
		that I can utilize in board specific setup.
	=09
		The way I suspect this is supposed to work is that from the board =
settup files I would do
		rtc_dev =3D platform_device_register_simple("pcf8563", -1, NULL, 0);=20
		which should later trigger the calling of the pcf8563_probe() =
function.
	=09
		This is doing things in the same way as the fsl i2c code, i.e.
		i2c_dev =3D platform_device_register_simple("i2c", i, r, 2);
		which by the way does not work untill I have converted the fsl_i2c =
(i2c-mpc.c) driver to the OF structure.
	=09
		So still the method of gluing together the OF drivers and non OF =
drivers eludes me.
	=09
		rg
		kd
	=09
		P.S. I did check the 2.6.21-RC7-git3 and found that the i2c-mpc.c and =
the rtc-pcf85763.c are basically the same
		as what I am working with in 2.6.20+
		________________________________
	=09
		From: John Rigby [mailto: jcrigby@gmail.com]
		Sent: 3. j=FAl=ED 2007 16:31
		To: K=E1ri Dav=ED=F0sson
		Cc: linuxppc-embedded@ozlabs.org=20
		Subject: Re: OF devices and non OF devices
	=09
	=09
		One place to find binding between OF devices and non OF devices is in =
arch/powerpc/sysdev/fsl_soc.c=20
		The typical pattern is:
		    if of_find_compatible_node "of-device-name"
		        platform_device_register_simple ""platform-device-name"
		        platform_device_add_data ...
	=09
	=09
	=09
		On 7/3/07, K=E1ri Dav=ED=F0sson <kari.davidsson@marel.is> wrote:
	=09
		        Hi,
	=09
		        I am attempting to get some non OF devices working for an mpc =
5200 board, in particular=20
		        PCF8563 RTC.
	=09
		        This device has an non OF device interface which I believe is =
correct. After all it should work
		        on non OF platforms.
	=09
		        I have managed to get the board to run the i2c initialization =
(and probe) for the fsl-mpc i2c driver by=20
		        converting the fsl-mpc i2c driver to OF driver (I found some =
patch here that I based this work on).
	=09
	=09
		fsl-i2c is one of the devices handled by fsl_soc.c so you shouldn't =
need to change anything to=20
		make it work in the latest kernel.  CONFIG_FSL_SOC was only added to =
lite5200_defconfig recently so
		that may explain why it's not on in your kernel.
	=09
	=09
	=09
		        Since the PCF8563 driver is not OF driver only its =
initaliziation code is run but the .probe function=20
		        of the driver is never run. Basically (as far as I can =
understand) the .probe is never run because the
		        driver is not an OF driver.
	=09
		        I could convert the PCF8563 driver to OF driver and make it =
work for our puposes but I feel this is=20
		        1) Wrong
		        2) therefore wasted work.
	=09
	=09
		Since the driver must run on non OF platforms then it should not be =
converted.  You just need to add a platform_device_register somewhere.
		I don't think fsl_soc.c is  the right place since it is not part of an =
freescale SOC.=20
		You could probably put it in a board specific startup routine.
	=09
	=09
	=09
		        What seems to elude me is some glue that glues together the OF =
part of the driver space to the non OF part
		        of the driver space.=20
	=09
		        Any hints or pointers on where to find this glue?
	=09
		        Regards,
		        kd
	=09
		        P.S. Kernel is post 2.6.20.
	=09
		        --
		        K=E1ri Dav=ED=F0sson                   | =
kari.davidsson@marel.is
		        Hugb=FAna=F0arger=F0                   | www.marel.com
		        Tel: 563-8156 Fax: +354 563 8001=20
		        Iceland
		        _______________________________________________=20
		        Linuxppc-embedded mailing list
		        Linuxppc-embedded@ozlabs.org
		         https://ozlabs.org/mailman/listinfo/linuxppc-embedded =
<https://ozlabs.org/mailman/listinfo/linuxppc-embedded>=20
	=09
	=09
	=09
		_______________________________________________
		Linuxppc-embedded mailing list
		Linuxppc-embedded@ozlabs.org
		https://ozlabs.org/mailman/listinfo/linuxppc-embedded
	=09


=09

^ permalink raw reply

* Re: mpc52xx: Correct calculation of FEC RX errors???
From: Sylvain Munaut @ 2007-07-18 11:45 UTC (permalink / raw)
  To: Miguel Angel Alvarez; +Cc: linuxppc-embedded
In-Reply-To: <469DFC03.9080201@ziv.es>


> Hi
>
> We are showing figures of more than 4 billion error frames in our
> ethernet interfaces. We have tested that the problem is in a
> substraction (the number of errors decrements with the number of frames).
>
> So... looking in the fec driver (fec.c) for the calculations we have
> seen that the number of multicast packets is added to the number of
> correct frames in order to get the frame errors...
>
> But the interesting thing is that we have checked that this
> calculation is something that we have added with a patch by Grzegorz
> Bernacki in this list.
>
> So... The funny thing is... Why a patch that solves the problem for
> Grzegorz produces "the same problem" for us?
>
> And... by the way... I have seen IEEE802.3, and when they talk about
> aFramesReceivedOK (which I suppose is the ieee_r_frame_ok in the
> driver), and they do not say a word about not including multicast
> packets in it...
>
> Any comment will be appreciate.
The only comment I have, is that yes, the computation are flawed.
And that's not very high in my priority list.

I don't think the path posted on the list fully fix the issue but I
really don't want to spend hours trying to figure out exactly what
values are reported in all those counters. You're welcome to do so if
you have some free time ...

There are other stuff wrong in this driver (try ifconfig eth0 down, then
send some broad cast traffic on the network .... you'll see some fifo
error popping up ) ...


Sylvain

^ permalink raw reply

* mpc52xx: Correct calculation of FEC RX errors???
From: Miguel Angel Alvarez @ 2007-07-18 11:39 UTC (permalink / raw)
  To: linuxppc-embedded; +Cc: tnt

Hi

We are showing figures of more than 4 billion error frames in our=20
ethernet interfaces. We have tested that the problem is in a=20
substraction (the number of errors decrements with the number of =
frames).

So... looking in the fec driver (fec.c) for the calculations we have=20
seen that the number of multicast packets is added to the number of=20
correct frames in order to get the frame errors...

But the interesting thing is that we have checked that this calculation=20
is something that we have added with a patch by Grzegorz Bernacki in=20
this list.

So... The funny thing is... Why a patch that solves the problem for=20
Grzegorz produces "the same problem" for us?

And... by the way... I have seen IEEE802.3, and when they talk about=20
aFramesReceivedOK (which I suppose is the ieee_r_frame_ok in the=20
driver), and they do not say a word about not including multicast=20
packets in it...

Any comment will be appreciate.

Miguel =C1ngel =C1lvarez
**=20
=20
----------------------------------------- PLEASE NOTE =
-------------------------------------------
This message, along with any attachments, may be confidential or legally =
privileged.=20
It is intended only for the named person(s), who is/are the only =
authorized recipients.
If this message has reached you in error, kindly destroy it without =
review and notify the sender immediately.
Thank you for your help.
ZIV uses virus scanning software but excludes any liability for viruses =
contained in any attachment.
=20
------------------------------------ ROGAMOS LEA ESTE TEXTO =
-------------------------------
Este mensaje y sus anexos pueden contener informaci=F3n confidencial y/o =
con derecho legal.=20
Est=E1 dirigido =FAnicamente a la/s persona/s o entidad/es rese=F1adas =
como =FAnico destinatario autorizado.
Si este mensaje le hubiera llegado por error, por favor elim=EDnelo sin =
revisarlo ni reenviarlo y notif=EDquelo inmediatamente al remitente. =
Gracias por su colaboraci=F3n. =20
ZIV utiliza software antivirus, pero no se hace responsable de los virus =
contenidos en los ficheros anexos.

^ permalink raw reply

* Re: [PATCH v2] Allow exec on 32-bit from readable, non-exec pages, with a warning.
From: Segher Boessenkool @ 2007-07-18 12:07 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org, Jon Loeliger, Paul Mackerras
In-Reply-To: <3198C3F2-91CE-4AF6-822B-5BA2820458C7@kernel.crashing.org>

>>>> A:  They haven't been posted yet.
>>>>
>>>> Q:  How do we know Segher has new patches?
>>>
>>> He sent it to me to test, and I told him it worked...
>>
>> And I sent it to the list hours later, over a week ago.
>
> Can someone send a ozlabs linuxppc list link or patchworks to the  
> "new" patch.

<http://ozlabs.org/pipermail/linuxppc-dev/2007-July/039076.html>
is Johannes' ack.
It seems ozlabs ate the original, will resend.


Segher

^ permalink raw reply

* Re: [PATCH 38/61] cpm2: Update device trees.
From: Vitaly Bordug @ 2007-07-18 12:22 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013549.GJ15238@ld0162-tx32.am.freescale.net>

On Tue, 17 Jul 2007 20:35:50 -0500
Scott Wood wrote:

> +				             "fsl,cpm-uart";
>  				reg = <11a00 20 8000 100>;
> -				current-speed = <1c200>;
Hmm, how is it supposed to work without speed? I was testing my u-boot OF bindings for 82xx and spotted into it.


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [RFC 0/3] lro: Generic Large Receive Offload for TCP traffic (IPv6)
From: Jan-Bernd Themann @ 2007-07-18 13:00 UTC (permalink / raw)
  To: David Miller
  Cc: tklein, themann, netdev, linux-kernel, hch, linuxppc-dev, raisch,
	meder, stefan.roscher
In-Reply-To: <20070715.024015.126137834.davem@davemloft.net>

Hi,

I suggest we keep the interface open for IPv6 support by adding 
an additional parameter but first just get IPv4 support only 
into the kernel. IPv6 support can then incrementially be added.
Would that be ok?



On Sunday 15 July 2007 11:40, David Miller wrote:
> From: Christoph Hellwig <hch@infradead.org>
> Date: Sun, 15 Jul 2007 10:12:53 +0100
> 
> > I'm not sure that's a good idea.  If current chips can't handle ipv6
> > lro there is no way to actually test it and the code will surely bitrot.
> 
> Christoph, you can do LRO pretty much completely in software.
> 
> 

^ permalink raw reply

* [PATCH] PPC:  Prevent direct inclusion of <asm/rwsem.h>.
From: Robert P. J. Day @ 2007-07-18 13:36 UTC (permalink / raw)
  To: Linux PPC Mailing List; +Cc: Paul Mackerras


Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

---

  not compile-tested, so if there's a flaw here somewhere, feel free
to tweak it.

 arch/ppc/syslib/ocp.c       |    2 +-
 include/asm-powerpc/rwsem.h |    4 ++++
 include/asm-ppc/ocp.h       |    2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/ppc/syslib/ocp.c b/arch/ppc/syslib/ocp.c
index 491fe9a..3f5be2c 100644
--- a/arch/ppc/syslib/ocp.c
+++ b/arch/ppc/syslib/ocp.c
@@ -44,11 +44,11 @@
 #include <linux/pm.h>
 #include <linux/bootmem.h>
 #include <linux/device.h>
+#include <linux/rwsem.h>

 #include <asm/io.h>
 #include <asm/ocp.h>
 #include <asm/errno.h>
-#include <asm/rwsem.h>
 #include <asm/semaphore.h>

 //#define DBG(x)	printk x
diff --git a/include/asm-powerpc/rwsem.h b/include/asm-powerpc/rwsem.h
index e929145..cefc147 100644
--- a/include/asm-powerpc/rwsem.h
+++ b/include/asm-powerpc/rwsem.h
@@ -1,6 +1,10 @@
 #ifndef _ASM_POWERPC_RWSEM_H
 #define _ASM_POWERPC_RWSEM_H

+#ifndef _LINUX_RWSEM_H
+#error "Please don't include <asm/rwsem.h> directly, use <linux/rwsem.h> instead."
+#endif
+
 #ifdef __KERNEL__

 /*
diff --git a/include/asm-ppc/ocp.h b/include/asm-ppc/ocp.h
index 16dbc7d..1379a4f 100644
--- a/include/asm-ppc/ocp.h
+++ b/include/asm-ppc/ocp.h
@@ -27,10 +27,10 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/device.h>
+#include <linux/rwsem.h>

 #include <asm/mmu.h>
 #include <asm/ocp_ids.h>
-#include <asm/rwsem.h>
 #include <asm/semaphore.h>

 #ifdef CONFIG_PPC_OCP
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply related

* Re: [Bugme-new] [Bug 8778] New: Ocotea board: kernel reports access of bad area during boot with DEBUG_SLAB=y
From: Josh Boyer @ 2007-07-18 13:41 UTC (permalink / raw)
  To: Eugene Surovegin
  Cc: bart.vanassche, netdev, Andrew Morton,
	bugme-daemon@kernel-bugs.osdl.org, linuxppc-embedded
In-Reply-To: <20070718083425.GA29722@gate.ebshome.net>

On Wed, 2007-07-18 at 01:34 -0700, Eugene Surovegin wrote:
> On Wed, Jul 18, 2007 at 12:52:53AM -0700, Andrew Morton wrote:
> > On Wed, 18 Jul 2007 00:07:50 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:
> > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=8778
> > > 
> > >            Summary: Ocotea board: kernel reports access of bad area during
> > >                     boot with DEBUG_SLAB=y
> 
> Slab debugging is probably the culprit here. I had similar problem 
> couple of years ago, not sure something has changed since then, 
> haven't checked.
> 
> When slab debugging was enabled it made memory allocations non L1 
> cache line aligned. This is very bad for DMA on non-coherent cache 
> arches (PPC440 is one of those archs).
> 
> I have a hack for EMAC which tries to "workaround" this problem:
> 	http://kernel.ebshome.net/emac_slab_debug.diff
> which might help.

Would you be opposed to including that patch in mainline?  I'd like to
have the bug reporter try it and then get it in if it fixes the issue.

josh

^ permalink raw reply

* Re: [RFC][PATCH 6/8] Walnut DTS
From: Segher Boessenkool @ 2007-07-18 13:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Yoder Stuart-B08248
In-Reply-To: <1184707558.25235.159.camel@localhost.localdomain>

>> Yes, I shouldn't say "defaulted" -- a unit interrupt specifier
>> simply has no unit address part, in an interrupt domain that
>> doesn't correspond to a "normal" bus.  But saying it like this
>> is a little bit inexact, and it uses more words.
>>
>>> which is why I tend to prefer having it
>>> explicitely in the interrupt controller node :-)
>>
>> Which is simply incorrect.
>
> It's absolutely not.

Of course it is.  #address-cells is a property for
bus nodes, you don't go around putting it in unrelated
places, just like you don't put "slot-names" properties
where it has no business.

Oh, wait, you're going to say that is a stupid comparison.
Yeah it is, "slot-names" would be way more harmless, #a is
such a fundamental low-level building block of the OF device
tree that the imap people didn't dare change it semantics
(or, were simply smart enough to not try).

> Please, stop that moronic pin-head behaviour and
> find me a single case where that would actually be a problem of any =20=

> sort
> or form.

You want all DTS authors to put properties where they do
not belong, confusing them even further about what all this
stuff is.  You want to make this a "requirement", although
it of course won't help anything anywhere, since *even you
_do_ have to follow the rules*, because the Linux code has
to support proper standards-compliant OF device trees, too.

>> You mean, the magic default values you used for #address-cells
>> and #size-cells?  That was simply a bug, someone forgot to read
>> the documentation...
>
> No, defaults are crap, period. This is a general thing.

I'm sorry if you feel that way, but there simply is no
way you can do anything about it; defaults exist, you
have to support them.

> Besides, the
> spec itself has issues about the default values (remember those blurbs
> about PCI and ISA supposedly having different defaults ?)

I remember those very very vaguely.  The conclusion was that
people have trouble understanding what they read, if I
remember it right.

Anyway, quit saying vague things about supposed bugs in the
OF documents without pointing them out; it is less than
helpful.

> In any way,
> defaults are a bad idea and I'm happy to say don't use them.

Sure, feel free to recommend people to explicitly write out
default values.  Nothing wrong there, it's just one particular
coding style.

As I've explained enough times though, the issue at hand isn't
about writing out a default value though; you want to require
(not recommend) a completely bogus property where it doesn't
belong.  And none of the code will benefit, either.  I don't
see the point.

>> For this?  No way:
>>
>> [=46rom the base spec]:
>>
>> =93#address-cells=94 S
>> 	Standard property name to define the package=92s address format.
>> 	prop-encoded-array: Integer, encoded with encode-int.
>>
>> 	This property applies to packages that define a physical
>> 	address space, i.e., those packages with =93decode-unit=94
>> 	methods. The property value specifies the number of cells
>> 	that are used to encode a physical address within that
>> 	address space. The value of this property affects the other
>> 	functions, commands, and methods that deal with physical
>> 	addresses. In a package with a =93decode-unit=94 method, a =
missing
>> 	=93#address-cells=94 property signifies that the number of
>> 	address cells is two.
>
> And you omit the various bus bindings that have come up with different
> defaults...

a) Do you expect me to quote every OF document in every post?
    That would easily exceed the message limits on this list,
    sorry.
b) There are no such bindings.  Stop confusing people.  There
    might be a bug in one such binding or so, but that would be
    just that, a bug; or maybe it uses confusing language; or
    maybe some people just don't know how to read [that would
    include me, I'm not trying to escalate this, ahem, polite
    conversation].  In any case, please point out the exact
    spot where you say this happens.

>> See?  The flat device tree unfortunately has no decode-unit, but
>> it is still pretty clear which nodes "define a physical address
>> space" and which do not.
>>
>> There is nothing badly defined here.
>
> See above. Besides, as I said, default values are crap.

Hey, maybe if you repeat it often enough, I might start to
believe it.  Not that that will help your point, since it
is absolutely irrelevant.

> And no, it's not
> obvious which nodes define a physical address space or not, at =20
> least not
> for a generic parser.

"Everything which can have kids with an @ in your DTB full
name thingie".  That includes everything with a #address-cells.

> Defaults are a bad idea, just get it

I'm getting tired of repeating myself, I hope you do, too.

> and move on and stop arguing just for the sake of arguing.

Would you please stop using that childish argument when
you've run out of real ones?

> Pointing out the letter
> of the spec is not a constructive attitude here.

Not just the letter, also the spirit, and the history.

And of course it _is_ constructive; you want to make a
useless, non-compatible change, are we peasants allowed
to quote the law that even you the mighty king must follow?
It's the only weapon we have you know.  Oh, if you discount
the pitchforks ;-)

>> Nothing in the "interrupt mapping" spec redefines #address-cells
>> (OF isn't all that stupid you know); it simply says that a /unit
>> interrupt specifier/ has no /unit address/ part if there is no
>> #address-cells.  The algorithm in paragraph 7 makes it super
>> clear how exactly this should work.
>
> No, the algorithm provided isn't clear and is buggy.

Yes it is clear.  No it isn't buggy (it _does_ state right
before the algo that some details are left out, maybe that
hurt you?)

>  I have implemented it so I know what I'm talking about.

Me too.

> The fact that basically you end up
> with "different" defaults for what is essentially the value
> #address-cells depending on whether you are walking the device-tree =20=

> for
> address resolution or for interrupt resolution is stupid.

Until you realise #address-cells is missing in those two
cases for completely different reasons.  In the first case,
#address-cells is missing for historic reasons; there didn't
use to be a #address-cells, everyone implicitly assumed it
was 2, it wasn't a default as such.  After #a was introduced,
not specifying it if it was 2 had to be allowed, for compatibility
reasons.  To this day, people routinely write trees with implicit
#address-cells =3D <2>, simply since they find it handy [<-- this
is the only thing you can fight with that "defaults are bad"
argument, btw, and you won't get far; most people feel passionately
about their own coding style].

In the interrupt "tree" case though, #address-cells is missing
in certain nodes ***because it simply doesn't belong there!***
The imap spec doesn't put any new rules on #address-cells [how
could it, without risking serious damage], it simply specifies
how to deal with the existing bus tree structure.  And not every
node in the interrupt graph is a bus node, so it says how to
form a unit interrupt specifier when there is no #address-cells:
you simply leave out the unit part.  Wow.  Now that was hard.
And there is no "default" (in your sense) in sight.

> Thus, the
> solution is simple: don't do defaults. Explicit values are good.

Except where the decisive thing is whether there _is_ a value
at all.

>>> and the spec contains gray areas
>>> and contradictions as to what the default values should be in some
>>> circumstances.
>>
>> In some areas, perhaps.  And it would be nice to bring those
>> areas to the attention of the working group, instead of just
>> to complain.
>
> The working group is dead

"It's just resting".

> and some of the ex members of it expressed
> their lack of interest in pursuing these matters.

Some, yes.  Even if the WG wouldn't help, there are some other
forums where you can discuss OF bugs; if all else fails, you
could try this mailing list even.

Point remains: it is a terribly bad argument to say "defaults
are bad, ooh I'm creating a rumour there are some vague defaults
in the OF standard right here, without pointing out anything,
ever".


[Here you cut out the part of our little chat where you
suggest the interrupt mapping spec should have said that
if there is no #address-cells in a node, just put something
there]

>> If it would, the interrupt mapping spec would have had to say
>> how the semantics of #address-cells were changed (and they
>> weren't, and they shouldn't, and this is such a laughable idea
>> I wonder why anyone would suggest it did).
>
> That's bullshit. The semantics are exactly the same. You obviously
> decided to be immune to any kind of common sense today.

Huh?  Let me write more abstractly what I said:

	"If you change A, A gets changed".

Hard to argue with that I would think, but hey.

>> What the interrupt mapping spec defines is how to _use_ the
>> value of #address-cells, and how to interpret its absence;
>> what should be put in #address-cells for separate nodes is
>> defined elsewhere (namely, in the base spec, and in relevant
>> device bindings).
>
> There is no such crackpot interpretation.

Well please read it again, if that's what you think.  Obviously
you didn't understand it on the first try.  This is really basic
stuff; it's how standards work.

> A unit interrupt specifier
> contains ... an address. An address format/size is defined by a
> #address-cells.

No.  A unit address format is defined by its device binding.
Luckily, for (PCI) interrupt mapping, nothing more than the
unit address _size_ is needed since the imap spec doesn't
concern itself with the semantics of the unit addresses in
question; it just needs some way to get unique identifiers.

> Period. That's not an "interpretation", that's the
> basic, primary semantic of #address-cells.

Pretty much, yes...

> The fact that the absence of
> #address-cells will give a different "default" for the address size
> depending on "how" you walk the tree is just a plain wrong bad idea.

...so I don't see how you made _this_ mental jump.

There _is_ no default, there is just a different way to
form unit interrupt specifiers for nodes with or without
#address-cells.  They are treated differently, because
they _are_ different.  Yes you can try to shoehorn them
into one mould by pretending #address-cells exists and
has the value 0; this might be okay for an interrupt tree
walker implementation [or it might not -- the writers of
the example algorithm that you so despise probably had
a reason for writing it out all three times, even though
they aren't all that careful in other places], maybe it
isn't too confusing there, but it certainly is if you put
it in a device tree.

> I
> see no reason why it would be or cause or be in any way shape or form
> wrong or against the "spirit" of the spec (if not the letter) to
> explicitely specify in the case of leaf interrupt controllers, that
> their #address-cells is 0 and be done with it.

Maybe you don't see it.  But the specs make abundantly
clear that _not_ putting an #address-cells here is perfectly
fine, which I thought would have prevented this particular
flame war from happening at all.  Oh well.


Segher

^ permalink raw reply

* Re: [RFC][PATCH 6/8] Walnut DTS
From: Segher Boessenkool @ 2007-07-18 13:53 UTC (permalink / raw)
  To: Scott Wood; +Cc: Yoder Stuart-B08248, linuxppc-dev
In-Reply-To: <20070717222507.GA4682@ld0162-tx32.am.freescale.net>

>> See above. Besides, as I said, default values are crap. And no,  
>> it's not
>> obvious which nodes define a physical address space or not, at  
>> least not
>> for a generic parser.
>
> The obvious way (which indeed isn't what the suggested algorithm  
> does --
> but the suggested algorithm doesn't do anything sensible) is that  
> if you
> got to the node via an interrupt-parent or interrupt-map, it  
> doesn't use
> #address-cells, and if you got to it by going to the regular device  
> tree
> parent, it does.
>
> Pretty much any time you use the unit address in a context other  
> than the
> bus parent, things cease making sense.

Yes indeed.  The problem with your suggested "obvious way"
is that you wouldn't get a unit address included if your
interrupt-map points (for some entry) at your device tree
parent, either.  Not all that hypothetical.


Segher

^ permalink raw reply

* Re: compile error if CONFIG_BLOCK not enabled related to linux/ide.h include
From: Kumar Gala @ 2007-07-18 14:12 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <200707181247.26262.bzolnier@gmail.com>


On Jul 18, 2007, at 5:47 AM, Bartlomiej Zolnierkiewicz wrote:

>
> Hi,
>
> On Wednesday 18 July 2007, Kumar Gala wrote:
>> M:      bzolnier@gmail.com
>> L:      linux-ide@vger.kernel.org
>>
>> We get the following compile error if CONFIG_BLOCK isn't enabled:
>>
>>   CC      arch/powerpc/kernel/setup_32.o
>> In file included from arch/powerpc/kernel/setup_32.c:14:
>> include/linux/ide.h:558: error: expected specifier-qualifier-list  
>> before 'request_queue_t'
>> include/linux/ide.h:696: warning: 'struct request' declared inside  
>> parameter list
>> include/linux/ide.h:696: warning: its scope is only this  
>> definition or declaration, which is probably not what you want
>> include/linux/ide.h:820: warning: 'struct request' declared inside  
>> parameter list
>> include/linux/ide.h:853: error: field 'wrq' has incomplete type
>> include/linux/ide.h:1205: error: expected ')' before '*' token
>> make[1]: *** [arch/powerpc/kernel/setup_32.o] Error 1
>> make: *** [arch/powerpc/kernel] Error 2
>
> include/linux/ide.h is exclusively for IDE subsystem
>
>> What I'm trying to figure out is if include/linux/ide.h should be  
>> wrapped
>> in a #if defined(CONFIG_IDE) || defined(CONFIG_IDE_MODULE) or if  
>> there is
>> some other desired way to handle this.
>
> Plese remove <linux/ide.h> include, there doesn't seem to be any  
> piece of
> code in arch/powerpc/kernel/setup_32.c which needs it anyway.

Its needed for:

struct ide_machdep_calls ppc_ide_md;

which gets defined in asm/ide.h which needs linux/ide.h for the defn  
of ide_init_hwif.

- k

^ permalink raw reply

* Re: [PATCH v2] Allow exec on 32-bit from readable, non-exec pages, with a warning.
From: Kumar Gala @ 2007-07-18 14:13 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev@ozlabs.org, Jon Loeliger, Paul Mackerras
In-Reply-To: <F803407D-9A94-4662-8008-6ACF70837BDD@kernel.crashing.org>


On Jul 18, 2007, at 7:07 AM, Segher Boessenkool wrote:

>>>>> A:  They haven't been posted yet.
>>>>>
>>>>> Q:  How do we know Segher has new patches?
>>>>
>>>> He sent it to me to test, and I told him it worked...
>>>
>>> And I sent it to the list hours later, over a week ago.
>>
>> Can someone send a ozlabs linuxppc list link or patchworks to the  
>> "new" patch.
>
> <http://ozlabs.org/pipermail/linuxppc-dev/2007-July/039076.html>
> is Johannes' ack.
> It seems ozlabs ate the original, will resend.

Thanks, I know I remember seeing the ack but never the original patch :)

- k

^ permalink raw reply

* Re: [PATCH] PPC:  Prevent direct inclusion of <asm/rwsem.h>.
From: Kumar Gala @ 2007-07-18 14:14 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Linux PPC Mailing List, Paul Mackerras
In-Reply-To: <Pine.LNX.4.64.0707180934550.20961@localhost.localdomain>


On Jul 18, 2007, at 8:36 AM, Robert P. J. Day wrote:

>
> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

Can you provide some commit description as to why we are doing this?

- k

>
> ---
>
>   not compile-tested, so if there's a flaw here somewhere, feel free
> to tweak it.
>
>  arch/ppc/syslib/ocp.c       |    2 +-
>  include/asm-powerpc/rwsem.h |    4 ++++
>  include/asm-ppc/ocp.h       |    2 +-
>  3 files changed, 6 insertions(+), 2 deletions(-)

^ permalink raw reply

* Re: [PATCH] PPC:  Prevent direct inclusion of <asm/rwsem.h>.
From: Robert P. J. Day @ 2007-07-18 14:20 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Linux PPC Mailing List, Paul Mackerras
In-Reply-To: <C5BB127A-BE08-41D9-A516-2F309A1E2E52@kernel.crashing.org>

On Wed, 18 Jul 2007, Kumar Gala wrote:

>
> On Jul 18, 2007, at 8:36 AM, Robert P. J. Day wrote:
>
> >
> > Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
>
> Can you provide some commit description as to why we are doing this?
>
> - k
>
> >
> > ---
> >
> >   not compile-tested, so if there's a flaw here somewhere, feel free
> > to tweak it.
> >
> >  arch/ppc/syslib/ocp.c       |    2 +-
> >  include/asm-powerpc/rwsem.h |    4 ++++
> >  include/asm-ppc/ocp.h       |    2 +-
> >  3 files changed, 6 insertions(+), 2 deletions(-)

based on the pattern across all architectures, it would seem that
asm/rwsem.h is not *meant* to be included directly, but only via
linux/rwsem.h.  i'm not sure if that's the kind of rationale you're
looking for, or if you want a more technical description.

at the moment, ppc is the only architecture that is doing the above,
but it may be that there's a good reason for that that i'm just not
aware of.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Jan Engelhardt @ 2007-07-18 14:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Alessandro Rubini,
	linux-kernel, linuxppc-dev, Paul Mackerras, Jens Axboe
In-Reply-To: <20070716162206.392129000@pademelon.sonytel.be>


On Jul 16 2007 18:15, Geert Uytterhoeven wrote:
>
>Add a Disk Storage Driver for the PS3:
>  - Implemented as a block device driver with a dynamic major
>  - Disk names (and partitions) are of the format ps3d%c(%u)
>  - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
>    doesn't support scatter-gather

I wonder what virtualization has to do with a block device driver?


	Jan
-- 

^ permalink raw reply

* Re: [PATCH 28/61] Add cpm2_set_pin().
From: Kumar Gala @ 2007-07-18 15:02 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013535.GZ15238@ld0162-tx32.am.freescale.net>


On Jul 17, 2007, at 8:35 PM, Scott Wood wrote:

> This provides a generic way for board code to set up CPM pins, rather
> than directly poking magic values into registers.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  arch/powerpc/sysdev/cpm2_common.c |   28 ++++++++++++++++++++++++++++
>  include/asm-ppc/cpm2.h            |    8 ++++++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/ 
> sysdev/cpm2_common.c
> index 7e3a983..0a62df7 100644
> --- a/arch/powerpc/sysdev/cpm2_common.c
> +++ b/arch/powerpc/sysdev/cpm2_common.c
> @@ -338,3 +338,31 @@ void *cpm_dpram_addr(unsigned long offset)
>  	return (void *)(im_dprambase + offset);
>  }
>  EXPORT_SYMBOL(cpm_dpram_addr);
> +
> +struct cpm2_ioports {
> +	u32	dir, par, sor, odr, dat;

__be32?

> +	u32	res[3];
> +};
> +
> +void cpm2_set_pin(int port, int pin, int flags)

Can we make the function take a pointer to the port directly?

> +{
> +	struct cpm2_ioports __iomem *iop =
> +		(struct cpm_ioports __iomem *)&cpm2_immr->im_ioport;
> +
> +	pin = 1 << (31 - pin);
> +
> +	if (flags & CPM_PIN_OUTPUT)
> +		setbits32(&iop[port].dir, pin);
> +	else
> +		clrbits32(&iop[port].dir, pin);
> +
> +	if (!(flags & CPM_PIN_GPIO))
> +		setbits32(&iop[port].par, pin);
> +	else
> +		clrbits32(&iop[port].par, pin);
> +
> +	if (flags & CPM_PIN_SECONDARY)
> +		setbits32(&iop[port].sor, pin);
> +	else
> +		clrbits32(&iop[port].sor, pin);

should we only do this if !(flags & CPM_PIN_GPIO)?

Any reason we don't also set odr here?

> +}
> diff --git a/include/asm-ppc/cpm2.h b/include/asm-ppc/cpm2.h
> index 12a2860..0e7ffb6 100644
> --- a/include/asm-ppc/cpm2.h
> +++ b/include/asm-ppc/cpm2.h
> @@ -1244,5 +1244,13 @@ enum cpm_clk {
>
>  extern int cpm2_clk_setup(enum cpm_clk_target target, int clock,  
> int mode);
>
> +#define CPM_PIN_INPUT     0
> +#define CPM_PIN_OUTPUT    1
> +#define CPM_PIN_PRIMARY   0
> +#define CPM_PIN_SECONDARY 2
> +#define CPM_PIN_GPIO      4
> +
> +void cpm2_set_pin(int port, int pin, int flags);
> +
>  #endif /* __CPM2__ */
>  #endif /* __KERNEL__ */
> -- 
> 1.5.0.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC
From: Segher Boessenkool @ 2007-07-18 15:30 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Johannes Berg

Some old software on ppc32 executes from pages it hasn't marked
executable.  Since "classic" hardware doesn't distinguish between
execute and read accesses, the do_page_fault() code shouldn't
either.  This makes glibc-2.2 work again on such hardware.

Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
---
[Resend again, ozlabs' greylisting doesn't like me at all.]

Tested by Scott on 32-bit, glibc-2.2.5 and glibc-2.3.3 (no new
failures and problem solved), and by Johannes on his glibc-2.4
"---p" testcase.  Could use testing on ppc64 and BookE too, for
good measure.

This reverts the previous change and makes the bugfix behave
more like the arch/ppc code.
 arch/powerpc/mm/fault.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 115b25f..5d7add0 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -278,14 +278,17 @@ good_area:
 		goto bad_area;
 #endif /* CONFIG_8xx */
 
+#ifdef CONFIG_PPC64
 	if (is_exec) {
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
 		/* protection fault */
 		if (error_code & DSISR_PROTFAULT)
 			goto bad_area;
 		if (!(vma->vm_flags & VM_EXEC))
 			goto bad_area;
-#else
+	} else
+		/* A read or write, code continues below...  */
+#elsif defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
+	if (is_exec) {
 		pte_t *ptep;
 		pmd_t *pmdp;
 
@@ -310,9 +313,12 @@ good_area:
 			}
 			pte_unmap_unlock(ptep, ptl);
 		}
+	} else
+		/* A read or write, code continues below...  */
 #endif
-	/* a write */
-	} else if (is_write) {
+
+	/* A read or write.  Classic PPC32 execute is considered a read.  */
+	if (is_write) {
 		if (!(vma->vm_flags & VM_WRITE))
 			goto bad_area;
 	/* a read */
-- 
1.5.2.1.144.gabc40-dirty

^ permalink raw reply related

* [PATCH] eHEA: Fix bonding support
From: Thomas Klein @ 2007-07-18 15:34 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, Stefan Roscher, linux-ppc, Jan-Bernd Themann,
	Marcus Eder

The driver didn't allow an interface's MAC address to be modified if the
respective interface wasn't setup - a failing Hcall was the result. Thus
bonding wasn't usable. The fix moves the failing Hcall which was registering
a MAC address for the reception of BC packets in firmware from the port up
and down functions to the port resources setup functions. Additionally the
missing update of the last_rx member of the netdev structure was added.

Signed-off-by: Thomas Klein <tklein@de.ibm.com>

---
 drivers/net/ehea/ehea.h      |    2 +-
 drivers/net/ehea/ehea_main.c |   37 +++++++++++++++++++------------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 6628fa6..489c8b2 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -39,7 +39,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"ehea"
-#define DRV_VERSION	"EHEA_0070"
+#define DRV_VERSION	"EHEA_0071"
 
 /* eHEA capability flags */
 #define DLPAR_PORT_ADD_REM 1
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 1d1571c..4c70a93 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -466,6 +466,8 @@ static struct ehea_cqe *ehea_proc_rwqes(struct net_device *dev,
 							 cqe->vlan_tag);
 			else
 				netif_receive_skb(skb);
+
+			dev->last_rx = jiffies;
 		} else {
 			pr->p_stats.poll_receive_errors++;
 			port_reset = ehea_treat_poll_error(pr, rq, cqe,
@@ -1433,7 +1435,8 @@ static int ehea_broadcast_reg_helper(struct ehea_port *port, u32 hcallid)
 				     port->logical_port_id,
 				     reg_type, port->mac_addr, 0, hcallid);
 	if (hret != H_SUCCESS) {
-		ehea_error("reg_dereg_bcmc failed (tagged)");
+		ehea_error("%sregistering bc address failed (tagged)",
+                           hcallid == H_REG_BCMC ? "" : "de");
 		ret = -EIO;
 		goto out_herr;
 	}
@@ -1444,7 +1447,8 @@ static int ehea_broadcast_reg_helper(struct ehea_port *port, u32 hcallid)
 				     port->logical_port_id,
 				     reg_type, port->mac_addr, 0, hcallid);
 	if (hret != H_SUCCESS) {
-		ehea_error("reg_dereg_bcmc failed (vlan)");
+		ehea_error("%sregistering bc address failed (vlan)",
+			   hcallid == H_REG_BCMC ? "" : "de");
 		ret = -EIO;
 	}
 out_herr:
@@ -2170,7 +2174,6 @@ static int ehea_up(struct net_device *dev)
 {
 	int ret, i;
 	struct ehea_port *port = netdev_priv(dev);
-	u64 mac_addr = 0;
 
 	if (port->state == EHEA_PORT_UP)
 		return 0;
@@ -2189,18 +2192,10 @@ static int ehea_up(struct net_device *dev)
 		goto out_clean_pr;
 	}
 
-	ret = ehea_broadcast_reg_helper(port, H_REG_BCMC);
-	if (ret) {
-		ret = -EIO;
-		ehea_error("out_clean_pr");
-		goto out_clean_pr;
-	}
-	mac_addr = (*(u64*)dev->dev_addr) >> 16;
-
 	ret = ehea_reg_interrupts(dev);
 	if (ret) {
-		ehea_error("out_dereg_bc");
-		goto out_dereg_bc;
+		ehea_error("reg_interrupts failed. ret:%d", ret);
+		goto out_clean_pr;
 	}
 
 	for(i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
@@ -2226,9 +2221,6 @@ static int ehea_up(struct net_device *dev)
 out_free_irqs:
 	ehea_free_interrupts(dev);
 
-out_dereg_bc:
-	ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
-
 out_clean_pr:
 	ehea_clean_all_portres(port);
 out:
@@ -2273,7 +2265,6 @@ static int ehea_down(struct net_device *dev)
 				&port->port_res[i].d_netdev->state))
 			msleep(1);
 
-	ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
 	port->state = EHEA_PORT_DOWN;
 
 	ret = ehea_clean_all_portres(port);
@@ -2655,12 +2646,18 @@ struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
 
 	INIT_WORK(&port->reset_task, ehea_reset_port);
 
+	ret = ehea_broadcast_reg_helper(port, H_REG_BCMC);
+	if (ret) {
+		ret = -EIO;
+		goto out_unreg_port;
+	}
+
 	ehea_set_ethtool_ops(dev);
 
 	ret = register_netdev(dev);
 	if (ret) {
 		ehea_error("register_netdev failed. ret=%d", ret);
-		goto out_unreg_port;
+		goto out_dereg_bc;
 	}
 
 	ret = ehea_get_jumboframe_status(port, &jumbo);
@@ -2675,6 +2672,9 @@ struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
 
 	return port;
 
+out_dereg_bc:
+	ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
+
 out_unreg_port:
 	ehea_unregister_port(port);
 
@@ -2694,6 +2694,7 @@ static void ehea_shutdown_single_port(struct ehea_port *port)
 {
 	unregister_netdev(port->netdev);
 	ehea_unregister_port(port);
+	ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
 	kfree(port->mc_list);
 	free_netdev(port->netdev);
 	port->adapter->active_ports--;
-- 
1.5.2

^ permalink raw reply related

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Geert Uytterhoeven @ 2007-07-18 15:36 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Alessandro Rubini,
	linux-kernel, linuxppc-dev, Paul Mackerras, Jens Axboe
In-Reply-To: <Pine.LNX.4.64.0707181632200.5385@fbirervta.pbzchgretzou.qr>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1170 bytes --]

On Wed, 18 Jul 2007, Jan Engelhardt wrote:
> On Jul 16 2007 18:15, Geert Uytterhoeven wrote:
> >Add a Disk Storage Driver for the PS3:
> >  - Implemented as a block device driver with a dynamic major
> >  - Disk names (and partitions) are of the format ps3d%c(%u)
> >  - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> >    doesn't support scatter-gather
> 
> I wonder what virtualization has to do with a block device driver?

On the PS3, storage devices are virtualized by the hypervisor (yes, the PS3
runs all OSes under a hypervisor).

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: [PATCH 10/61] bootwrapper: flatdevtree fixes
From: Milton Miller @ 2007-07-18 15:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson

In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>

On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 1. ft_create_node was returning the internal pointer rather than a 
> phandle.
> 2. ft_find_device_rel was treating lookups relative to root as an 
> error.

No, it is treating lookups relative to NULL as an error.

Your patch changes it to treat lookups relative to the NULL phandle as 
relative to root.

I've no objections to the other part, can you split these?

>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
>  arch/powerpc/boot/flatdevtree.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/flatdevtree.c 
> b/arch/powerpc/boot/flatdevtree.c
> index b732644..5b69aeb 100644
> --- a/arch/powerpc/boot/flatdevtree.c
> +++ b/arch/powerpc/boot/flatdevtree.c
> @@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt, 
> const void *top,
>  {
>         char *node;
>
> -       node = ft_node_ph2node(cxt, top);
> -       if (node == NULL)
> -               return NULL;
> +       if (top) {
> +               node = ft_node_ph2node(cxt, top);
> +               if (node == NULL)
> +                       return NULL;
> +       } else {
> +               node = ft_root_node(cxt);
> +       }
>

milton

^ permalink raw reply

* Re: [PATCH 09/61] bootwrapper: Add dt_is_compatible().
From: Milton Miller @ 2007-07-18 15:42 UTC (permalink / raw)
  To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson

In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>

On Wed Jul 18 11:33:06 EST 2007, Scott Wood wrote:
> +int dt_is_compatible(void *node, const char *compat)
> +{
> +       char *buf = (char *)prop_buf;
> +       int compat_len = strlen(compat);
> +       int len, pos;
> +
> +       len = getprop(node, "compatible", buf, MAX_PROP_LEN);
> +       if (len < 0)
> +               return 0;
> +
> +       for (pos = 0; pos + compat_len < len; pos++) {
> +               if (!strcmp(buf + pos, compat))
> +                       return 1;
> +
> +               while (buf[pos] && pos + compat_len < len)
> +                       pos++;

This is buggy: if you are searching for "ns16550" and the compatable is 
"fsl,1234\0commons16550" this code will incorrectly says its 
compatable.

Comparing pos < len instead will do the right thing, at the cost of a 
few iterations of the loop.

> +       }
> +
> +       return 0;

milton

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox