| Message ID | 1311167599-21790-1-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 20, 2011 at 09:13:19PM +0800, Shawn Guo wrote: > The UART_PADDR definition for i.mx53 and i.mx50 is missing in > debug-macro.S. It causes the build of i.mx53/50 fail. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> > --- > Troy, > > Since I'm keen to get the build failure fixed, I just repost > the patch you sent with Sascha's comment fixed. Hope you do > not mind. Then you should at least add his Signed-off. And maybe also mark him as the author of the patch?
On Wed, Jul 20, 2011 at 03:08:30PM +0200, Wolfram Sang wrote: > On Wed, Jul 20, 2011 at 09:13:19PM +0800, Shawn Guo wrote: > > The UART_PADDR definition for i.mx53 and i.mx50 is missing in > > debug-macro.S. It causes the build of i.mx53/50 fail. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> > > --- > > Troy, > > > > Since I'm keen to get the build failure fixed, I just repost > > the patch you sent with Sascha's comment fixed. Hope you do > > not mind. > > Then you should at least add his Signed-off. And maybe also mark him as > the author of the patch? > I did exactly what you suggested here on a mx53 fec patch, but I was told by Troy to change his s-o-b to reported-by. So let's see what he would say about this one.
> > Then you should at least add his Signed-off. And maybe also mark him as > > the author of the patch? > > > I did exactly what you suggested here on a mx53 fec patch, but I was > told by Troy to change his s-o-b to reported-by. So let's see what > he would say about this one. Okay, not much of a deal for such a patch. Though, I have doubts if one can request removing the SoB for a patch other people put work on top of.
On Wed, Jul 20, 2011 at 03:08:30PM +0200, Wolfram Sang wrote: > On Wed, Jul 20, 2011 at 09:13:19PM +0800, Shawn Guo wrote: > > The UART_PADDR definition for i.mx53 and i.mx50 is missing in > > debug-macro.S. It causes the build of i.mx53/50 fail. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> > > --- > > Troy, > > > > Since I'm keen to get the build failure fixed, I just repost > > the patch you sent with Sascha's comment fixed. Hope you do > > not mind. > > Then you should at least add his Signed-off. And maybe also mark him as > the author of the patch? > Actually, I was just building the imx53 and making the fix (I hope you agree I do not need to steal Troy's code for such a quick and easy fix), and then recall there is a patch already on the list.
On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: > > > Then you should at least add his Signed-off. And maybe also mark him as > > > the author of the patch? > > > > > I did exactly what you suggested here on a mx53 fec patch, but I was > > told by Troy to change his s-o-b to reported-by. So let's see what > > he would say about this one. > > Okay, not much of a deal for such a patch. Though, I have doubts if one > can request removing the SoB for a patch other people put work on top > of. > So you are telling you are not following the list closely? http://permalink.gmane.org/gmane.linux.network/200918
On Wed, Jul 20, 2011 at 09:44:27PM +0800, Shawn Guo wrote: > On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: > > > > Then you should at least add his Signed-off. And maybe also mark him as > > > > the author of the patch? > > > > > > > I did exactly what you suggested here on a mx53 fec patch, but I was > > > told by Troy to change his s-o-b to reported-by. So let's see what > > > he would say about this one. > > > > Okay, not much of a deal for such a patch. Though, I have doubts if one > > can request removing the SoB for a patch other people put work on top > > of. > > > So you are telling you are not following the list closely? If "following closely" == "reading every single mail", then surely not. Why?
On Wed, Jul 20, 2011 at 03:45:30PM +0200, Wolfram Sang wrote: > On Wed, Jul 20, 2011 at 09:44:27PM +0800, Shawn Guo wrote: > > On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: > > > > > Then you should at least add his Signed-off. And maybe also mark him as > > > > > the author of the patch? > > > > > > > > > I did exactly what you suggested here on a mx53 fec patch, but I was > > > > told by Troy to change his s-o-b to reported-by. So let's see what > > > > he would say about this one. > > > > > > Okay, not much of a deal for such a patch. Though, I have doubts if one > > > can request removing the SoB for a patch other people put work on top > > > of. > > > > > So you are telling you are not following the list closely? > > If "following closely" == "reading every single mail", then surely not. > Why? > Well, not really. But since you are co-maintaining i.mx, I *thought* you may have read the mail with "i.mx" in the subject. It seems not the case, apparently.
On 7/20/2011 6:45 AM, Wolfram Sang wrote: > On Wed, Jul 20, 2011 at 09:44:27PM +0800, Shawn Guo wrote: >> On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: >>>>> Then you should at least add his Signed-off. And maybe also mark him as >>>>> the author of the patch? >>>>> >>>> I did exactly what you suggested here on a mx53 fec patch, but I was >>>> told by Troy to change his s-o-b to reported-by. So let's see what >>>> he would say about this one. >>> >>> Okay, not much of a deal for such a patch. Though, I have doubts if one >>> can request removing the SoB for a patch other people put work on top >>> of. >>> >> So you are telling you are not following the list closely? > > If "following closely" == "reading every single mail", then surely not. > Why? > Shawn, The reason I requested to change to reported-by on the FEC patch was because your patch was extremely different from mine and you deserved to be listed as the author, not me. For this, both patches are tiny. And since I still think that CONFIG_ARCH_MX53 is more appropriate than CONFIG_SOC_IMX53, I am also fine with a reported-by. Though your commit message could be better. It is a run time problem, not compile-time. Thanks for your efforts. Troy
On Wed, Jul 20, 2011 at 09:59:22PM +0800, Shawn Guo wrote: > On Wed, Jul 20, 2011 at 03:45:30PM +0200, Wolfram Sang wrote: > > On Wed, Jul 20, 2011 at 09:44:27PM +0800, Shawn Guo wrote: > > > On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: > > > > > > Then you should at least add his Signed-off. And maybe also mark him as > > > > > > the author of the patch? > > > > > > > > > > > I did exactly what you suggested here on a mx53 fec patch, but I was > > > > > told by Troy to change his s-o-b to reported-by. So let's see what > > > > > he would say about this one. > > > > > > > > Okay, not much of a deal for such a patch. Though, I have doubts if one > > > > can request removing the SoB for a patch other people put work on top > > > > of. > > > > > > > So you are telling you are not following the list closely? > > > > If "following closely" == "reading every single mail", then surely not. > > Why? > > > Well, not really. But since you are co-maintaining i.mx, I *thought* > you may have read the mail with "i.mx" in the subject. It seems not > the case, apparently. ? I don't get the relevance to the topic, sorry. If the new patch is different enough to be considered new, Troy could indeed request his SoB to be removed. If his patch is mainly improved, I do still wonder if he could request his SoB to be removed. That is possibly a gray area, but as I said before, it really doesn't matter for this patch and is not worth the fuzz here. Let's just go back to hacking :)
On 7/20/2011 6:13 AM, Shawn Guo wrote: > The UART_PADDR definition for i.mx53 and i.mx50 is missing in > debug-macro.S. It causes the build of i.mx53/50 fail. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> > --- > Troy, > > Since I'm keen to get the build failure fixed, I just repost > the patch you sent with Sascha's comment fixed. Hope you do > not mind. > > arch/arm/plat-mxc/include/mach/debug-macro.S | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S > index 91fc7cd..8cf8dee 100644 > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S > @@ -44,6 +44,14 @@ > #define UART_PADDR MX51_UART1_BASE_ADDR > #endif > > +#if defined(CONFIG_SOC_IMX50) || defined(CONFIG_SOC_IMX53) > +#ifdef UART_PADDR > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs" > +#endif > +/* i.MX50 gets the same UART1 base address as i.MX53 */ > +#define UART_PADDR MX53_UART1_BASE_ADDR > +#endif > + If your debug-macro.S is like mine then you should have #ifdef CONFIG_ARCH_MX5 #ifdef UART_PADDR #error "CONFIG_DEBUG_LL is incompatible with multiple archs" #endif #define UART_PADDR MX51_UART1_BASE_ADDR #endif So, all MX5's are using MX51_UART1_BASE_ADDR. I can't see that your patch changes CONFIG_ARCH_MX5 to CONFIG_SOC_IMX51. It isn't functionally the same as my version. > #define UART_VADDR IMX_IO_ADDRESS(UART_PADDR) > > .macro addruart, rp, rv
On Wed, Jul 20, 2011 at 01:13:20PM -0700, Troy Kisky wrote: > On 7/20/2011 6:45 AM, Wolfram Sang wrote: > > On Wed, Jul 20, 2011 at 09:44:27PM +0800, Shawn Guo wrote: > >> On Wed, Jul 20, 2011 at 03:27:20PM +0200, Wolfram Sang wrote: > >>>>> Then you should at least add his Signed-off. And maybe also mark him as > >>>>> the author of the patch? > >>>>> > >>>> I did exactly what you suggested here on a mx53 fec patch, but I was > >>>> told by Troy to change his s-o-b to reported-by. So let's see what > >>>> he would say about this one. > >>> > >>> Okay, not much of a deal for such a patch. Though, I have doubts if one > >>> can request removing the SoB for a patch other people put work on top > >>> of. > >>> > >> So you are telling you are not following the list closely? > > > > If "following closely" == "reading every single mail", then surely not. > > Why? > > > Shawn, > > The reason I requested to change to reported-by on the FEC patch was > because your patch was extremely different from mine and you deserved to > be listed as the author, not me. > > > For this, both patches are tiny. And since I still think that > CONFIG_ARCH_MX53 is more appropriate than CONFIG_SOC_IMX53, > I am also fine with a reported-by. Though your commit message > could be better. It is a run time problem, not compile-time. > No. It is a compile-time problem. The reason we see difference there is because I based off linux-next (essentially linux-arm-soc/next <- Sascha's 'devel' branch), while you probably based off Sascha's for-next branch. Sascha, You forgot to apply the following patch on your for-next branch? commit fad107086d5a869c1c07e5bb35b7b57a10ecf578 Author: Sascha Hauer <s.hauer@pengutronix.de> Date: Thu May 19 17:25:05 2011 +0200 ARM i.MX debug macro: use CONFIG_SOC_* instead of CONFIG_ARCH_* CONFIG_ARCH_* are deprecated, so remove one user. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> I think you still need to keep your for-next branch as a bleeding-edge for i.mx since people base their i.mx works off there.
On Wed, Jul 20, 2011 at 02:10:37PM -0700, Troy Kisky wrote: > On 7/20/2011 6:13 AM, Shawn Guo wrote: > > The UART_PADDR definition for i.mx53 and i.mx50 is missing in > > debug-macro.S. It causes the build of i.mx53/50 fail. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> > > --- > > Troy, > > > > Since I'm keen to get the build failure fixed, I just repost > > the patch you sent with Sascha's comment fixed. Hope you do > > not mind. > > > > arch/arm/plat-mxc/include/mach/debug-macro.S | 8 ++++++++ > > 1 files changed, 8 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S > > index 91fc7cd..8cf8dee 100644 > > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S > > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S > > @@ -44,6 +44,14 @@ > > #define UART_PADDR MX51_UART1_BASE_ADDR > > #endif > > > > +#if defined(CONFIG_SOC_IMX50) || defined(CONFIG_SOC_IMX53) > > +#ifdef UART_PADDR > > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs" > > +#endif > > +/* i.MX50 gets the same UART1 base address as i.MX53 */ > > +#define UART_PADDR MX53_UART1_BASE_ADDR > > +#endif > > + > > If your debug-macro.S is like mine then you should have > No, it is not. > #ifdef CONFIG_ARCH_MX5 > #ifdef UART_PADDR > #error "CONFIG_DEBUG_LL is incompatible with multiple archs" > #endif > #define UART_PADDR MX51_UART1_BASE_ADDR > #endif > > > So, all MX5's are using MX51_UART1_BASE_ADDR. > I can't see that your patch changes CONFIG_ARCH_MX5 > to CONFIG_SOC_IMX51. It isn't functionally the same as my version. > We started from the difference base. No, it is not functionally the same as yours, so even remove your reported-by? :)
On 7/20/2011 4:16 PM, Shawn Guo wrote: > On Wed, Jul 20, 2011 at 02:10:37PM -0700, Troy Kisky wrote: >> On 7/20/2011 6:13 AM, Shawn Guo wrote: >> >> So, all MX5's are using MX51_UART1_BASE_ADDR. >> I can't see that your patch changes CONFIG_ARCH_MX5 >> to CONFIG_SOC_IMX51. It isn't functionally the same as my version. >> > We started from the difference base. No, it is not functionally the > same as yours, so even remove your reported-by? :) > Thanks for the explanation. You'll get no complaints from me. Do as you like. Troy
diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S index 91fc7cd..8cf8dee 100644 --- a/arch/arm/plat-mxc/include/mach/debug-macro.S +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S @@ -44,6 +44,14 @@ #define UART_PADDR MX51_UART1_BASE_ADDR #endif +#if defined(CONFIG_SOC_IMX50) || defined(CONFIG_SOC_IMX53) +#ifdef UART_PADDR +#error "CONFIG_DEBUG_LL is incompatible with multiple archs" +#endif +/* i.MX50 gets the same UART1 base address as i.MX53 */ +#define UART_PADDR MX53_UART1_BASE_ADDR +#endif + #define UART_VADDR IMX_IO_ADDRESS(UART_PADDR) .macro addruart, rp, rv
The UART_PADDR definition for i.mx53 and i.mx50 is missing in debug-macro.S. It causes the build of i.mx53/50 fail. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> Reported-by: Troy Kisky <troy.kisky@boundarydevices.com> --- Troy, Since I'm keen to get the build failure fixed, I just repost the patch you sent with Sascha's comment fixed. Hope you do not mind. arch/arm/plat-mxc/include/mach/debug-macro.S | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)