Patchwork arm/mxc: add the missing UART_PADDR for i.mx53

login
register
mail settings
Submitter Shawn Guo
Date July 20, 2011, 1:13 p.m.
Message ID <1311167599-21790-1-git-send-email-shawn.guo@linaro.org>
Download mbox | patch
Permalink /patch/105681/
State New
Headers show

Comments

Wolfram Sang - July 20, 2011, 1:08 p.m.
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?
Shawn Guo - July 20, 2011, 1:13 p.m.
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(-)
Shawn Guo - July 20, 2011, 1:24 p.m.
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.
Wolfram Sang - July 20, 2011, 1:27 p.m.
> > 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.
Shawn Guo - July 20, 2011, 1:40 p.m.
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.
Shawn Guo - July 20, 2011, 1:44 p.m.
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
Wolfram Sang - July 20, 2011, 1:45 p.m.
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 Guo - July 20, 2011, 1:59 p.m.
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.
Troy Kisky - July 20, 2011, 8:13 p.m.
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
Wolfram Sang - July 20, 2011, 8:44 p.m.
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 :)
Troy Kisky - July 20, 2011, 9:10 p.m.
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
Shawn Guo - July 20, 2011, 11:13 p.m.
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.
Shawn Guo - July 20, 2011, 11:16 p.m.
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? :)
Troy Kisky - July 21, 2011, 12:31 a.m.
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

Patch

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