Skip to contentSkip to conversation

<asm/mpc8xxx_spi.h> is only available for ppc boards.

PATCH New Peter Pearse 11 participants 24 messages Jan 24, 2008 – Mar 23, 2008 SPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

<asm/mpc8xxx_spi.h> is only available for ppc boards.

Patch against
git://www.denx.de/git/u-boot.git
commit 33dac03b1b5d61e4fed7bad445ba40b4c97feba0

---
diff -purN u-boot_unpatched/drivers/spi/mpc8xxx_spi.c u-boot/drivers/spi/mpc8xxx_spi.c
--- u-boot_unpatched/drivers/spi/mpc8xxx_spi.c 2008-01-24
16:18:07.375090000 +0000
+++ u-boot/drivers/spi/mpc8xxx_spi.c 2008-01-24 16:14:50.560475000 +0000
@@ -23,7 +23,13 @@

#include <common.h>
#include <spi.h>
+
+#if defined(CONFIG_MPC834X) || \

+	defined(CONFIG_MPC8313) || \
+	defined(CONFIG_MPC8315) || \

+ defined(CONFIG_MPC837X)
#include <asm/mpc8xxx_spi.h>
+#endif

#ifdef CONFIG_HARD_SPI
---

or should asm-ppc/mpc8xxx_spi.h be moved to drivers/spi?

Regards

Peter

#1 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 11 lines · Peter Pearse
(quoted diff) +5 -1
+#if defined(CONFIG_MPC834X) || \
+       defined(CONFIG_MPC8313) || \
+       defined(CONFIG_MPC8315) || \
+       defined(CONFIG_MPC837X)
 #include <asm/mpc8xxx_spi.h>
+#endif
 
 #ifdef CONFIG_HARD_SPI
---
Quoted 1 line

or should asm-ppc/mpc8xxx_spi.h be moved to drivers/spi?

Hm. I'd prefer that, since that SPI driver will possibly propagate through *many* variants, and it seems silly to change the driver every time we make a new part. :)

I'm not familiar enough with the device or driver to know whether the header is truly ppc-specific, or just coincidentally so.

Andy

#2 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 11 lines · On Thu, 24 Jan 2008 10:50:09 -0600 "Andy Fleming" <[email protected]> wrote:

Review

Quoted 10 lines · Peter Pearse
(quoted diff) +5 -1
+#if defined(CONFIG_MPC834X) || \
+       defined(CONFIG_MPC8313) || \
+       defined(CONFIG_MPC8315) || \
+       defined(CONFIG_MPC837X)
 #include <asm/mpc8xxx_spi.h>
+#endif
 
 #ifdef CONFIG_HARD_SPI
---

does Vlad's post not work for you?:

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/35888

Quoted 9 lines
Quoted 2 lines

or should asm-ppc/mpc8xxx_spi.h be moved to drivers/spi?

Hm. I'd prefer that, since that SPI driver will possibly propagate through *many* variants, and it seems silly to change the driver every time we make a new part. :)

I'm not familiar enough with the device or driver to know whether the header is truly ppc-specific, or just coincidentally so.

mpc8xxx_spi.h is included by other files in asm-ppc.

btw, apologies for not testing non-ppc.

Kim

#3 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 12 lines · Kim Phillips
Quoted 9 lines · On Thu, 24 Jan 2008 10:50:09 -0600 "Andy Fleming" <[email protected]> wrote:
Quoted 8 lines · Peter Pearse
(quoted diff) +5 -0
+#if defined(CONFIG_MPC834X) || \
+       defined(CONFIG_MPC8313) || \
+       defined(CONFIG_MPC8315) || \
+       defined(CONFIG_MPC837X)
 #include <asm/mpc8xxx_spi.h>
+#endif
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

#ifdef CONFIG_HARD_SPI
---

does Vlad's post not work for you?:

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/35888

Ping? Above bug is still present in current git. And while there what about fixing it this way? (controling mpc8xxx_spi compilation with CONFIG_HARD_SPI is a bit misleading, but lets fix it with separate patch. Any CONFIG_* name proposals?)

* Convert drivers/spi/Makefile to use CONFIG_
fixes "<asm/mpc8xxx_spi.h> is only available for ppc board" bug

drivers/spi/Makefile +1 -1
index 0b7a2df..6403a1c 100644
@@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
 
 LIB	:= $(obj)libspi.a
 
-COBJS-y += mpc8xxx_spi.o
+COBJS-$(CONFIG_HARD_SPI) += mpc8xxx_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/spi/mpc8xxx_spi.c +0 -3
index a3d1c95..5635047 100644
@@ -25,8 +25,6 @@
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
 
-#ifdef CONFIG_HARD_SPI
-
 #define SPI_EV_NE	(0x80000000 >> 22)	/* Receiver Not Empty */
 #define SPI_EV_NF	(0x80000000 >> 23)	/* Transmitter Not Full */
 
@@ -140,4 +138,3 @@ int spi_xfer(spi_chipsel_type chipsel, int bitlen, uchar *dout, uchar *din)
 
 	return 0;
 }
-#endif				/* CONFIG_HARD_SPI */
#4 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 17 lines · Ladislav Michl
Quoted 16 lines · Kim Phillips
Quoted 11 lines · On Thu, 24 Jan 2008 10:50:09 -0600 "Andy Fleming" <[email protected]> wrote:

Review

Quoted 10 lines · Peter Pearse
(quoted diff) +5 -1
+#if defined(CONFIG_MPC834X) || \
+       defined(CONFIG_MPC8313) || \
+       defined(CONFIG_MPC8315) || \
+       defined(CONFIG_MPC837X)
 #include <asm/mpc8xxx_spi.h>
+#endif
 
 #ifdef CONFIG_HARD_SPI
---

does Vlad's post not work for you?:

Quoted 7 lines
Quoted 1 line

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/35888

Ping? Above bug is still present in current git. And while there what about fixing it this way? (controling mpc8xxx_spi compilation with CONFIG_HARD_SPI is a bit misleading, but lets fix it with separate patch. Any CONFIG_* name proposals?)

Here's the patch that will be going in:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36039

I guess Wolfgang just hasn't had a chance to pull the 83xx tree.

CONFIG_HARD_SPI is intended to be used by multiple controllers, not just MPC_8XXX

Quoted 10 lines

* Convert drivers/spi/Makefile to use CONFIG_
fixes "<asm/mpc8xxx_spi.h> is only available for ppc board" bug

drivers/spi/Makefile +0 -0
index 0b7a2df..6403a1c 100644

drivers/spi/Makefile include $(TOPDIR)/config.mk · line 28

Quoted 5 lines
drivers/spi/Makefile +1 -1
@@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
 
 LIB    := $(obj)libspi.a
 
-COBJS-y += mpc8xxx_spi.o
+COBJS-$(CONFIG_HARD_SPI) += mpc8xxx_spi.o

Can't do this for the reason mentioned above.

Quoted 32 lines
drivers/spi/Makefile +0 -0
 COBJS  := $(COBJS-y)
 SRCS   := $(COBJS:.o=.c)
drivers/spi/mpc8xxx_spi.c +0 -4
index a3d1c95..5635047 100644
@@ -25,8 +25,6 @@
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
 
-#ifdef CONFIG_HARD_SPI
-
 #define SPI_EV_NE      (0x80000000 >> 22)      /* Receiver Not Empty */
 #define SPI_EV_NF      (0x80000000 >> 23)      /* Transmitter Not Full */
 
@@ -140,4 +138,3 @@ int spi_xfer(spi_chipsel_type chipsel, int bitlen, uchar *dout, uchar *din)
 
        return 0;
 }
-#endif                         /* CONFIG_HARD_SPI */
 
 
-------------------------------------------------------------------------

This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

#5 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 7 lines · Ben Warren

Here's the patch that will be going in:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36039

I guess Wolfgang just hasn't had a chance to pull the 83xx tree.

CONFIG_HARD_SPI is intended to be used by multiple controllers, not just MPC_8XXX

Okay, I just tried to use the same logic - no functional changes. What about this as a replacement of above mentioned patch? (note that we are trying to migrate towards COBJS-$(CONFIG_*) += file.o in Makefiles instead of #ifdefs inside each file)

* Convert drivers/spi/Makefile to use CONFIG_
fixes "<asm/mpc8xxx_spi.h> is only available for ppc board" bug

drivers/spi/Makefile +1 -1
index 0b7a2df..be62089 100644
@@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
 
 LIB	:= $(obj)libspi.a
 
-COBJS-y += mpc8xxx_spi.o
+COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/spi/mpc8xxx_spi.c +0 -3
index a3d1c95..5635047 100644
@@ -25,8 +25,6 @@
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
 
-#ifdef CONFIG_HARD_SPI
-
 #define SPI_EV_NE	(0x80000000 >> 22)	/* Receiver Not Empty */
 #define SPI_EV_NF	(0x80000000 >> 23)	/* Transmitter Not Full */
 
@@ -140,4 +138,3 @@ int spi_xfer(spi_chipsel_type chipsel, int bitlen, uchar *dout, uchar *din)
 
 	return 0;
 }
-#endif				/* CONFIG_HARD_SPI */
include/configs/MPC8349EMDS.h +1 -0
index 07f2f30..bb90c2d 100644
@@ -356,6 +356,7 @@
 #define CFG_I2C2_OFFSET		0x3100
 
 /* SPI */
+#define CONFIG_MPC8XXX_SPI
 #define CONFIG_HARD_SPI			/* SPI with hardware support */
 #undef CONFIG_SOFT_SPI			/* SPI bit-banged */
 
#6 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 6 lines · Ladislav Michl
Quoted 5 lines · Ben Warren

Here's the patch that will be going in:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36039

I guess Wolfgang just hasn't had a chance to pull the 83xx tree.

Quoted 8 lines
Quoted 2 lines

CONFIG_HARD_SPI is intended to be used by multiple controllers, not just MPC_8XXX

Okay, I just tried to use the same logic - no functional changes. What about this as a replacement of above mentioned patch? (note that we are trying to migrate towards COBJS-$(CONFIG_*) += file.o in Makefiles instead of #ifdefs inside each file)

Yeah, man. I didn't know the build system supported this yet. Definitely preferable to conditionally compile a whole file than its contents.
If it works, add my

#7 lore raw

[U-Boot-Users] u-boot patch submission process

In reply to Ladislav Michl

I am working on a patch to add DM355 SoC support to u-boot git tree. I am following the steps listed at
http://www.denx.de/wiki/UBoot/DevelopmentProcess. Since the DM355 has an ARM9 processor, I am creating a patch for the u-boot-arm git tree.

Since one of the steps is to verify MAKEALL builds cleanly, I first ran MAKEALL before applying my patch to make sure all is well. Since I am focusing on ARM9, I only did a MAKEALL for ARM9 (I don't have a PCC toolchain). Specifically, I did:

git clone git://www.denx.de/git/u-boot-arm.git u-boot-arm cd u-boot-arm
CROSS_COMPILE=arm-linux- BUILD_DIR=/tmp/build MAKEALL_LOGDIR=/tmp/log ./MAKEALL ARM9

Much to my surprise, I not only got warnings, I got errors. The main error being:

mpc8xxx_spi.c:26:29: error: asm/mpc8xxx_spi.h: No such file or directory. I see a patch to fix this error has been submitted.

The second error I got relates to PUEN() / __REG2() macros. I am not sure if that is a real error or caused by the toolchain I am using. I looked at the code, but got lost with the use of __builtin_constant_p() in asm-arm/arch-pxa/hardware.h where __REG2 is defined as

# define __REG2(x,y)    \
        ( __builtin_constant_p(y) ? (__REG((x) + (y))) \
                                  : (*(volatile u32 *)((u32)&__REG(x) +
(y))) )

I have no way to check that any change I propose works so I don't feel comfortable providing a patch, (plus the above code reinforces my believe that accessing hardware should be made obvious using inl() outl() type API). Does anyone else see this error when using MAKEALL for ARM9?

Questions:

1) Did I do something wrong on how I invoked MAKEALL or maybe have a toolchain issue? Should MAKEALL ARM9 build cleanly?

2) I am looking at providing two patchs. One to add DM355 SoC support and a second to add DFU USB functionality to the u-boot git tree. Should I post the USB patch based on u-boot-arm git tree or the u-boot-usb git tree. I have to have DM355 SoC support in order to test the DFU USB functionality so I would rather have the patches apply to the same git tree. The USB code in both git trees is nearly identical. Any suggestions?

3) There are common USB UDC API definitions in usbdcore_omap1510.h and usbdcore_mpc8xx.h (and they don't agree on the API). I am adding usbdcore_musb.h that supports the same API. Should I pull out this common code in a separate patch or as part of the USB UDC support for DM355 patch?

The API I found to be common includes:

int udc_init(void);
void udc_irq(void);
int udc_endpoint_write(struct usb_endpoint_instance *endpoint); void udc_setup_ep(struct usb_device_instance *device, unsigned int ep,

                  struct usb_endpoint_instance *endpoint);

void udc_connect(void);
void udc_disconnect(void);
void udc_enable(struct usb_device_instance *device); void udc_disable(void);
void udc_startup_events(struct usb_device_instance *device); void udc_set_nak(int epid);
void udc_unset_nak (int epid);

Note that omap has udc_endpoint_write() not returning a value, but drivers/serial/usbtty.c expects udc_endpoint_write() to return a result code.

Basically I am confused on how to submit patches when one patch requires another patch and the patches could be applied to different git custodian trees.

Thanks,

Todd

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080210/e97fa897/attachment.htm

#8 lore raw

[U-Boot-Users] u-boot patch submission process

Quoted 37 lines · Todd Fischer

I am working on a patch to add DM355 SoC support to u-boot git tree. I am following the steps listed at
http://www.denx.de/wiki/UBoot/DevelopmentProcess. Since the DM355 has an ARM9 processor, I am creating a patch for the u-boot-arm git tree.

Since one of the steps is to verify MAKEALL builds cleanly, I first ran MAKEALL before applying my patch to make sure all is well. Since I am focusing on ARM9, I only did a MAKEALL for ARM9 (I don't have a PCC toolchain). Specifically, I did:

git clone git://www.denx.de/git/u-boot-arm.git u-boot-arm cd u-boot-arm
CROSS_COMPILE=arm-linux- BUILD_DIR=/tmp/build MAKEALL_LOGDIR=/tmp/log ./MAKEALL ARM9

Much to my surprise, I not only got warnings, I got errors. The main error being:

mpc8xxx_spi.c:26:29: error: asm/mpc8xxx_spi.h: No such file or directory. I see a patch to fix this error has been submitted.

The second error I got relates to PUEN() / __REG2() macros. I am not sure if that is a real error or caused by the toolchain I am using. I looked at the code, but got lost with the use of __builtin_constant_p() in asm-arm/arch-pxa/hardware.h where __REG2 is defined as

# define __REG2(x,y)    \
        ( __builtin_constant_p(y) ? (__REG((x) + (y))) \
                                  : (*(volatile u32 *)((u32)&__REG(x) + 
(y))) )

I have no way to check that any change I propose works so I don't feel comfortable providing a patch, (plus the above code reinforces my believe that accessing hardware should be made obvious using inl() outl() type API). Does anyone else see this error when using MAKEALL for ARM9?

Quoted 4 lines

Questions:

1) Did I do something wrong on how I invoked MAKEALL or maybe have a toolchain issue?

Regarding the spi.h most probably not.

Quoted 1 line

Should MAKEALL ARM9 build cleanly?

No. The u-boot-arm.git is broken. I got the (unfortunately) private answer from Peter (ARM custodian) regarding

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36022/

-- cut --
Yup - my mail with subject

[U-Boot-Users] [PATCH] [DRIVERS] <asm/mpc8xxx_spi.h> is onlyavailable for ppc boards.

I'm assuming this will eventually be properly patched in the main tree
- meanwhile anyone building ARM code can apply this patch -- cut --

This is quite annoying, particularly if you should test patches already in that tree or against that. IMHO the arm.git should contain the fix temporarily to be able to build cleanly instead of staying with a broken tree.

To workaround I would propose that you test MAKEALL against mainline U-Boot git.

Quoted 7 lines

2) I am looking at providing two patchs. One to add DM355 SoC support and a second to add DFU USB functionality to the u-boot git tree. Should I post the USB patch based on u-boot-arm git tree or the u-boot-usb git tree. I have to have DM355 SoC support in order to test the DFU USB functionality so I would rather have the patches apply to the same git tree. The USB code in both git trees is nearly identical. Any suggestions?

Same as above: Test your patches against mainline U-Boot git and then send them as a series with anything like

[PATCH 1/2] ARM: DAVINCI: DM355 SoC
[PATCH 2/2] USB: DFU USB functionality

Nice to hear that you work on DM355.

Dirk

Quoted 32 lines

3) There are common USB UDC API definitions in usbdcore_omap1510.h and usbdcore_mpc8xx.h (and they don't agree on the API). I am adding usbdcore_musb.h that supports the same API. Should I pull out this common code in a separate patch or as part of the USB UDC support for DM355 patch?

The API I found to be common includes:

int udc_init(void);
void udc_irq(void);
int udc_endpoint_write(struct usb_endpoint_instance *endpoint); void udc_setup_ep(struct usb_device_instance *device, unsigned int ep,

                  struct usb_endpoint_instance *endpoint);

void udc_connect(void);
void udc_disconnect(void);
void udc_enable(struct usb_device_instance *device); void udc_disable(void);
void udc_startup_events(struct usb_device_instance *device); void udc_set_nak(int epid);
void udc_unset_nak (int epid);

Note that omap has udc_endpoint_write() not returning a value, but drivers/serial/usbtty.c expects udc_endpoint_write() to return a result code.

Basically I am confused on how to submit patches when one patch requires another patch and the patches could be applied to different git custodian trees.

Thanks,

Todd

#9 lore raw

[U-Boot-Users] u-boot patch submission process

Quoted 11 lines · On Mon, 11 Feb 2008 05:43:18 +0100 Dirk Behme <[email protected]> wrote:

[U-Boot-Users] [PATCH] [DRIVERS] <asm/mpc8xxx_spi.h> is onlyavailable for ppc boards.

I'm assuming this will eventually be properly patched in the main tree
- meanwhile anyone building ARM code can apply this patch -- cut --

This is quite annoying, particularly if you should test patches already in that tree or against that. IMHO the arm.git should contain the fix temporarily to be able to build cleanly instead of staying with a broken tree.

You're damn right it's annoying, but applying the fix to every single arch tree just isn't the right solution. That will cause mayhem when it turns out that every arch maintainer picked up a different fix among the 6 or so that were posted.

But if Wolfgang doesn't wake up and apply a fix pretty soon, I guess that might be exactly what's going to happen.

Quoted 2 lines

To workaround I would propose that you test MAKEALL against mainline U-Boot git.

Won't help since it's mainline that is broken. And it's not only ARM -- every single architecture except PPC has been broken for several weeks. The first fix was posted less than three hours after the breakage was introduced, but no fix has yet been merged.

I'm starting to get seriously pissed off about this whole situation and how it's been handled. Wolfgang, you can't with a straight face demand that people always run the latest git snapshot WHEN IT DOESN'T EVEN BUILD AND HAS BEEN THAT WAY FOR SEVERAL WEEKS!

Haavard

#10 lore raw

[U-Boot-Users] Broken (ARM) build, was: u-boot patch submission process

Quoted 22 lines · Haavard Skinnemoen
Quoted 11 lines · On Mon, 11 Feb 2008 05:43:18 +0100 Dirk Behme <[email protected]> wrote:

[U-Boot-Users] [PATCH] [DRIVERS] <asm/mpc8xxx_spi.h> is onlyavailable for ppc boards.

I'm assuming this will eventually be properly patched in the main tree
- meanwhile anyone building ARM code can apply this patch -- cut --

This is quite annoying, particularly if you should test patches already in that tree or against that. IMHO the arm.git should contain the fix temporarily to be able to build cleanly instead of staying with a broken tree.

You're damn right it's annoying, but applying the fix to every single arch tree just isn't the right solution. That will cause mayhem when it turns out that every arch maintainer picked up a different fix among the 6 or so that were posted.

Quoted 8 lines

But if Wolfgang doesn't wake up and apply a fix pretty soon, I guess that might be exactly what's going to happen.

Quoted 2 lines

To workaround I would propose that you test MAKEALL against mainline U-Boot git.

Won't help since it's mainline that is broken.

Uups, yes. Last time I tried mainline it worked, so I thought only arm.git has this issue.

But testing arm.git and mainline git from today, yes, *both* stop with

mpc8xxx_spi.c:26:29: error: asm/mpc8xxx_spi.h: No such file or directory

At least for (ARM based) davinci_dvevm_config I tried.

Sorry for the confusion.

Quoted 2 lines

And it's not only ARM --
every single architecture except PPC has been broken for several weeks.

Seems that we should asap have a fix for this.

Dirk

Quoted 10 lines

The first fix was posted less than three hours after the breakage was introduced, but no fix has yet been merged.

I'm starting to get seriously pissed off about this whole situation and how it's been handled. Wolfgang, you can't with a straight face demand that people always run the latest git snapshot WHEN IT DOESN'T EVEN BUILD AND HAS BEEN THAT WAY FOR SEVERAL WEEKS!

Haavard

#11 lore raw

[U-Boot-Users] u-boot patch submission process

In reply to Haavard Skinnemoen

Dear Haavard,

Quoted 3 lines · in message <20080211092625.4d09fe0b@siona> you wrote:

But if Wolfgang doesn't wake up and apply a fix pretty soon, I guess that might be exactly what's going to happen.

Please believe me, I'm not sleeping. I wish I was - and if it was only for 5 hours per night....

Quoted 4 lines

I'm starting to get seriously pissed off about this whole situation and how it's been handled. Wolfgang, you can't with a straight face demand that people always run the latest git snapshot WHEN IT DOESN'T EVEN BUILD AND HAS BEEN THAT WAY FOR SEVERAL WEEKS!

I'm pissedoff with the current situation myself, but I cannot work more than a certain number of hours per day, which unfortunately is not even close to 24. And I have to set some priorities.

Would it help if I apologized and explaine that I've been sick fro a few days, and that this lost ime just added to my backlog? No, I don;t want to do that and you don't want to hear that, all you want is some faster progress. So please do yourself and me a favour, don't increase the preassure on me and let me continue to work as fast as I can. Even if it's too slow.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"There was no difference between  the  behavior  of  a  god  and  the
operations of pure chance..."   - Thomas Pynchon, _Gravity's Rainbow_
#12 lore raw

[U-Boot-Users] u-boot patch submission process

Quoted 9 lines

-----Original Message-----
From: wd at denx.de [mailto:wd at denx.de]
Sent: 11 February 2008 23:25
To: Haavard Skinnemoen
Cc: Dirk Behme; todd.fischer at ridgerun.com; u-boot-users; Peter Pearse
Subject: Re: [U-Boot-Users] u-boot patch submission process

--- snip ---

Quoted 5 lines

So
please do yourself and me a favour, don't increase the preassure on me and let me continue to work as fast as I can. Even if it's too slow.

I'll vote for that

Regards

Peter

#13 lore raw

[U-Boot-Users] u-boot patch submission process

In reply to Wolfgang Denk
Quoted 9 lines · On Tue, 12 Feb 2008 00:25:26 +0100 Wolfgang Denk <[email protected]> wrote:

Dear Haavard,

Quoted 3 lines · in message <20080211092625.4d09fe0b@siona> you wrote:

But if Wolfgang doesn't wake up and apply a fix pretty soon, I guess that might be exactly what's going to happen.

Please believe me, I'm not sleeping. I wish I was - and if it was only for 5 hours per night....

Sorry. I know you work very hard.

Quoted 8 lines
Quoted 4 lines

I'm starting to get seriously pissed off about this whole situation and how it's been handled. Wolfgang, you can't with a straight face demand that people always run the latest git snapshot WHEN IT DOESN'T EVEN BUILD AND HAS BEEN THAT WAY FOR SEVERAL WEEKS!

I'm pissedoff with the current situation myself, but I cannot work more than a certain number of hours per day, which unfortunately is not even close to 24. And I have to set some priorities.

I didn't mean to imply that you should work more. But I did mean to complain about your priorities.

Quoted 6 lines

Would it help if I apologized and explaine that I've been sick fro a few days, and that this lost ime just added to my backlog? No, I don;t want to do that and you don't want to hear that, all you want is some faster progress. So please do yourself and me a favour, don't increase the preassure on me and let me continue to work as fast as I can. Even if it's too slow.

Yes, that would indeed help (although no apology is needed). All I've seen is that the tree has been broken, and you've been ignoring the patches. A short note saying that you're sick or just doesn't have the time to look at the patches would have helped a lot.

But to make things more confusing, I've seen you've applied other, less important patches while the tree was broken. If you had taken the time spent on one of those patches and applied one of the build fixes instead (doesn't matter much which one at this point -- we can always work out the details later), that would have helped even more.

As for the rate of progress, trying to pressure anyone to work faster is not a good idea, I know that. But is there any way we can change the process and move more work down the hierarchy? It seems to me you're doing pretty thorough review on everything that goes into the tree, and you're also sweeping up quite a few patches that the custodians didn't pick up. This does not scale very well...one of the most important responsibilities of a custodian is indeed review (or at least ensuring that someone else does it.)

Again, sorry for using such harsh words. But I do think we should discuss how to handle situations like this better in the future. Trying to prevent it from happening at all is not productive -- we all make mistakes -- but this means dealing with it in a timely manner is all the more important. And it doesn't just happen in u-boot; the avr32 architecture broke three times during the Linux 2.6.25 merge window (as did lots of other architectures). The difference is that the breakage was fixed almost instantly when I or someone else complained about it.

But note that whenever I'm complaining about patches not being applied, slow progress, or whatever, I'm really not complaining about _you_, I'm complaining about the process.

Haavard

#14 lore raw

[U-Boot-Users] u-boot patch submission process

Quoted 6 lines · In message <[email protected]> you wrote:

But to make things more confusing, I've seen you've applied other, less important patches while the tree was broken. If you had taken the time spent on one of those patches and applied one of the build fixes instead (doesn't matter much which one at this point -- we can always work out the details later), that would have helped even more.

When I scan patches, this requires that I not only check the patch, but all the following thread(s) it spawns. This is not only time-consuming, but has also good potential to lose track of what's been done and what not (yes, we are missing a patch tracking system, and yes, we are working on it - it's the highest priority of our admin here right now). The only chance for me to keep track is to go through more or less chronologically. That means that patcehs don't get applied in priority order (unless somebody explicitely triggers me), but sequentially.

Quoted 3 lines

As for the rate of progress, trying to pressure anyone to work faster is not a good idea, I know that. But is there any way we can change the process and move more work down the hierarchy? It seems to me you're

Yes, there is a way. I must get rid of the time-consuming task to check which patches have been picked up by some custodian, and which not, and if not, if they have been rejected are are ready to be applied or what. For this we need a patch tracking system, where we can see immediately who is "responsible" for a specific patch.

That's top prio for me. And we are working on it.

Quoted 5 lines

doing pretty thorough review on everything that goes into the tree, and you're also sweeping up quite a few patches that the custodians didn't pick up. This does not scale very well...one of the most important responsibilities of a custodian is indeed review (or at least ensuring that someone else does it.)

You are right, this is indeed the main problem. I don't want to let good patches disappear in a black hole just because nobody else cared about them. At the moment, all I can do is review them all.

Quoted 2 lines

Again, sorry for using such harsh words. But I do think we should discuss how to handle situations like this better in the future. Trying

Well, I've explained a few times before that we are working on a bug & patch tracking system. Maybe I should have been more expolicit how vitally important this is for my own work.

Quoted 6 lines

to prevent it from happening at all is not productive -- we all make mistakes -- but this means dealing with it in a timely manner is all the more important. And it doesn't just happen in u-boot; the avr32 architecture broke three times during the Linux 2.6.25 merge window (as did lots of other architectures). The difference is that the breakage was fixed almost instantly when I or someone else complained about it.

I'm always open for shortcuts - if I see something is urgent I'm willing to act ASAP. I just didn't see the 8xx SPI issue was so critical - I knew there was a patch in the queue, so what. I was wrong here. The most helpful posting was the one from Kim Phillips - he raised both my attention *and* provided a repo to pull from.

Quoted 3 lines

But note that whenever I'm complaining about patches not being applied, slow progress, or whatever, I'm really not complaining about _you_, I'm complaining about the process.

I understand this. But the process has bottlenecks, and sitting as a cork in such one while the pressure is building up below my *ss doesn't make me happy either ;-)

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I haven't lost my mind -- it's backed up on tape somewhere.
#15 lore raw

[U-Boot-Users] u-boot patch submission process

Wolfgang Denk wrote:

[snip]

Quoted 14 lines

When I scan patches, this requires that I not only check the patch, but all the following thread(s) it spawns. This is not only time-consuming, but has also good potential to lose track of what's been done and what not (yes, we are missing a patch tracking system, and yes, we are working on it - it's the highest priority of our admin here right now). The only chance for me to keep track is to go through more or less chronologically. That means that patcehs don't get applied in priority order (unless somebody explicitely triggers me), but sequentially.

Quoted 3 lines

As for the rate of progress, trying to pressure anyone to work faster is not a good idea, I know that. But is there any way we can change the process and move more work down the hierarchy? It seems to me you're

Quoted 7 lines

Yes, there is a way. I must get rid of the time-consuming task to check which patches have been picked up by some custodian, and which not, and if not, if they have been rejected are are ready to be applied or what. For this we need a patch tracking system, where we can see immediately who is "responsible" for a specific patch.

That's top prio for me. And we are working on it.

Jeremy Kerr has a perl script that picks out patches:
<http://ozlabs.org/~jk/projects/patchwork/>
<http://patchwork.ozlabs.org/> - try before you buy
I'm not sure how effective it is at following the threads. One VERY NICE thing about it is the "download" button that gives you a clean patch email. Unless I'm missing something, sites like gmane.org don't have a way of getting truly clean (unHTML-ized, no email obfuscation) versions of the archives.

Hmmm, Patchwork's threading capabilities appear to be less than gmane's
<http://thread.gmane.org/gmane.linux.ports.ppc.embedded/17429>
vs
<http://patchwork.ozlabs.org/linuxppc-embedded/patch?id=13060>
and
<http://patchwork.ozlabs.org/linuxppc-embedded/patch?id=13051>
Based on a very cursory review, it looks like a good idea, but needs more polishing before it makes it to version 1.0.

Testimonial:
<http://laforge.gnumonks.org/weblog/2005/08/index.html>

---------------------------------------------------------------------

For project management, I keep looking at Trac with the git plugin (not sure how well the git plugin works, but OLPC development uses it). My gut feel from browsing open source projects is that it is one of the more popular systems, but I don't know if it is a good match for u-boot/denx.de's needs.
<http://trac.edgewall.org/>
The user list is pretty extensive:
<http://trac.edgewall.org/wiki/TracUsers>

OLPC uses trac and git:
<http://dev.laptop.org/>

---------------------------------------------------------------------

Django is an interesting dB / Python interface, the same problem space addressed by Ruby on Rails.
<http://www.djangoproject.com/>

I have a vision of extending Trac, possibly through Django, to encompass more of the development lifecycle. In this discussion, for instance, rewriting Patchwork in python and then marrying it with Trac. Interestingly enough, Django uses Trac (and svn, but we won't hold that against it ;-).

[snipped the cork comment, much too graphic ;-]

Quoted 2 lines

Best regards,
Wolfgang Denk

Best regards,
gvb

#16 lore raw

[U-Boot-Users] u-boot patch submission process

Quoted 4 lines · In message <[email protected]> you wrote:

Jeremy Kerr has a perl script that picks out patches:
<http://ozlabs.org/~jk/projects/patchwork/>
<http://patchwork.ozlabs.org/> - try before you buy

Yes, I'm aware of this.

We're hacking gnats to do what we need :-)

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Any sufficiently advanced technology is indistinguishable from magic.
Clarke's Third Law       - _Profiles of the Future_ (1962; rev. 1973)
                  ``Hazards of Prophecy: The Failure of Imagination''
#17 lore raw

[U-Boot-Users] u-boot patch submission process

In reply to Dirk Behme
Quoted 9 lines

-----Original Message-----
From: Dirk Behme [mailto:dirk.behme at googlemail.com]
Sent: 11 February 2008 04:43
To: todd.fischer at ridgerun.com
Cc: u-boot-users; Peter Pearse
Subject: Re: [U-Boot-Users] u-boot patch submission process

Quoted 3 lines
Quoted 1 line · Todd Fischer

I am working on a patch to add DM355 SoC support to u-boot

git tree.

--- snip ---

Quoted 8 lines
Quoted 2 lines

Much to my surprise, I not only got warnings, I got errors.

The main

Quoted 5 lines

error being:

mpc8xxx_spi.c:26:29: error: asm/mpc8xxx_spi.h: No such file or directory. I see a patch to fix this error has been submitted.

--- snip ---

Quoted 4 lines

No. The u-boot-arm.git is broken. I got the (unfortunately) private answer from Peter (ARM custodian) regarding

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36022/

Sorry Dirk - wasn't supposed to be private.....

Todd,

Here's a patch that works, but probably isn't optimal, there was more discussion on the list,

but the correct solution hasn't been agreed yet.

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/35955

Regards

Peter

#18 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

In reply to Ladislav Michl
Quoted 14 lines · In message <[email protected]> you wrote:
Quoted 7 lines · Ben Warren

Here's the patch that will be going in:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36039

I guess Wolfgang just hasn't had a chance to pull the 83xx tree.

CONFIG_HARD_SPI is intended to be used by multiple controllers, not just MPC_8XXX

Okay, I just tried to use the same logic - no functional changes. What about this as a replacement of above mentioned patch? (note that we are trying to migrate towards COBJS-$(CONFIG_*) += file.o in Makefiles instead of #ifdefs inside each file)

Quoted 4 lines

* Convert drivers/spi/Makefile to use CONFIG_
fixes "<asm/mpc8xxx_spi.h> is only available for ppc board" bug

I'm unsure about the state of this patch. Please comment - if it's still missing, please rebase and resubmit.

Thanks.

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Today's robots are very primitive, capable of understanding  only  a
few  simple  instructions  such  as 'go left', 'go right', and 'build
car'."                                                  - John Sladek
#19 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

Quoted 15 lines · Wolfgang Denk
Quoted 14 lines · In message <[email protected]> you wrote:
Quoted 7 lines · Ben Warren

Here's the patch that will be going in:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36039

I guess Wolfgang just hasn't had a chance to pull the 83xx tree.

CONFIG_HARD_SPI is intended to be used by multiple controllers, not just MPC_8XXX

Okay, I just tried to use the same logic - no functional changes. What about this as a replacement of above mentioned patch? (note that we are trying to migrate towards COBJS-$(CONFIG_*) += file.o in Makefiles instead of #ifdefs inside each file)

Quoted 8 lines
Quoted 4 lines

* Convert drivers/spi/Makefile to use CONFIG_
fixes "<asm/mpc8xxx_spi.h> is only available for ppc board" bug

I'm unsure about the state of this patch. Please comment - if it's still missing, please rebase and resubmit.

My original patch fixed the problem and is in-tree. I'll submit one to move conditional compilation to Makefile from the code, but it's not a pressing matter.

regards,
Ben

#20 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

In reply to Ladislav Michl
Quoted 4 lines · On Fri, 8 Feb 2008 23:48:07 +0100 Ladislav Michl <[email protected]> wrote:

Ping? Above bug is still present in current git. And while there what about fixing it this way? (controling mpc8xxx_spi compilation with CONFIG_HARD_SPI is a bit misleading, but lets fix it with separate patch. Any CONFIG_* name proposals?)

Why not CONFIG_MPC8XXX_SPI?

I totally agree we need to get this fixed. Every single architecture except powerpc has been broken for weeks. This is just ridiculous.

Haavard

#21 lore raw

PATCH 1/1 <asm/mpc8xxx_spi.h> is only available for ppc boards.

In reply to Andy Fleming
Quoted 19 lines · Andy Fleming
Quoted 10 lines · Peter Pearse
(quoted diff) +5 -1
+#if defined(CONFIG_MPC834X) || \
+       defined(CONFIG_MPC8313) || \
+       defined(CONFIG_MPC8315) || \
+       defined(CONFIG_MPC837X)
 #include <asm/mpc8xxx_spi.h>
+#endif
 
 #ifdef CONFIG_HARD_SPI
---
Quoted 2 lines

or should asm-ppc/mpc8xxx_spi.h be moved to drivers/spi?

Hm. I'd prefer that, since that SPI driver will possibly propagate through *many* variants, and it seems silly to change the driver every time we make a new part. :)

Quoted 5 lines

I'm not familiar enough with the device or driver to know whether the header is truly ppc-specific, or just coincidentally so.

Andy

The header is specific to this SPI controller, but not really PPC-specific. I don't know enough about how Freescale works to know if the different divisions such as PPC and Coldfire and whatever else share building blocks like these controllers, but conceivably (in my mind at least) this could be on anything Freescale makes.

I'm no expert on Kconfig, but I think that once it's in play the Makefile for this driver determines whether to even compile the driver depending on whether a CONFIG is set or not. I probably forgot to define something like CONFIG_MPC8XXX_SPI. Maybe a change to the Makefile is needed here?

Sorry for the seemingly random mumblings.

regards,
Ben

#22 lore raw

[U-Boot-Users] ppc start.S 'rfmci' opcode not recognized by ppc-440 compiler

Hi,
I'm working on a amcc taishan board.
The board works find with for u-boot 1.1.3 and 1.1.6.
I'm using montavista ppc_440-gcc build tools.
But any version after 1.2.0 compilation fails at start.S apparently it complains about an Unrecognized opcode 'rfmci (message show below)

Questions:

1) also i made a change in the Makefile show above which made me wonder if using ppc_8xx as the default for all ppc board is a correct assumption ? maybe its just a place holder. or maybe the rfmci is only available in ppc_8xx.
2) other question is what option do i have to use the latest u-boot 1.3.1 with my ppc_440 build tools as it doesn't recognizes 'rfmci' opcode.

any insights would be appreciated.
Thanks in advance !
kunal

-----------------------------------
change that i made in Makefile
-----------------------------------

ifeq ($(ARCH),ppc)
CROSS_COMPILE = ppc_8xx-
endif
to
ifeq ($(ARCH),ppc)
CROSS_COMPILE = ppc_440-
endif

-----------------------------------------------------
following is the error message for make all
-----------------------------------------------------

c_440-objcopy -O srec hello_world hello_world.srec 2>/dev/null

ppc_440-ld -g  -Ttext 0x40000 \
                        -o sched -e sched sched.o libstubs.a \
                        -L/opt/montavista/pro/devkit/ppc/440/bin/../lib/gcc-lib/powerpc-hardhat-linux/3.3.1 -lgcc

ppc_440-objcopy -O srec sched sched.srec 2>/dev/null
ppc_440-objcopy -O binary hello_world hello_world.bin 2>/dev/null ppc_440-objcopy -O binary sched sched.bin 2>/dev/null
make[1]: Leaving directory `/export/old-root/export/share/uboot/u-boot-1.3.1/examples' make -C cpu/ppc4xx start.o
make[1]: Entering directory /export/old-root/export/share/uboot/u-boot-1.3.1/cpu/ppc4xx' make[1]: Leaving directory /export/old-root/export/share/uboot/u-boot-1.3.1/cpu/ppc4xx' make[1]: Entering directory `/export/old-root/export/share/uboot/u-boot-1.3.1/cpu/ppc4xx'
ppc_440-gcc -D__ASSEMBLY__ -g -Os -fPIC -ffixed-r14 -meabi -fno-strict-aliasing -D__KERNEL__ -DTEXT_BASE=0xFFFC0000 -I/export/old-root/export/share/uboot/u-boot-1.3.1/include -fno-builtin -ffreestanding -nostdinc -isystem /opt/montavista/pro/devkit/ppc/440/bin/../lib/gcc-lib/powerpc-hardhat-linux/3.3.1/include -pipe -DCONFIG_PPC -D__powerpc__ -DCONFIG_4xx -ffixed-r2 -ffixed-r29 -mstring -msoft-float -Wa,-m440 -mcpu=440 -DCONFIG_440=1 -c -o start.o start.S
start.S: Assembler messages:
start.S:1210: Error: Unrecognized opcode: `rfmci'
make[1]: *** [start.o] Error 1
make[1]: Leaving directory `/export/old-root/export/share/uboot/u-boot-1.3.1/cpu/ppc4xx'
make: *** [cpu/ppc4xx/start.o] Error 2

_________________________________________________________________
Shed those extra pounds with MSN and The Biggest Loser!
http://biggestloser.msn.com/

#23 lore raw

[U-Boot-Users] ppc start.S 'rfmci' opcode not recognized by ppc-440 compiler

Quoted 2 lines · In message <[email protected]> you wrote:

1) also i made a change in the Makefile show above which made me wonder if using ppc_8xx as the default for all ppc board is a correct assumption ?

It's a pretty good default, as you can build all existing PPC targets with it, and 90% of them will actually run on the target. Of course it's not recommended to actually do that. Instead, you should always set CXROSS_COMPILE as appropriate for your CPU.

Quoted 3 lines

maybe its just a place holder. or maybe the rfmci is only available in ppc_8xx.

2) other question is what option do i have to use the latest u-boot 1.3.1 with my ppc_440 build tools as it doesn't recognizes 'rfmci' opcode.

Please read the documentation for your toolchain, i. e. how to use it correctly. If this doesn't help, please contact MontaVista technical support.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Keep your eyes wide open before marriage, half shut afterwards.
                                                 -- Benjamin Franklin