[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Troy Kisky]

The i.MX51 UART_PADDR value does not work for MX50/53.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
index 8e8d175..3919ad4 100644
--- a/arch/arm/plat-mxc/include/mach/debug-macro.S
+++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
@@ -37,13 +37,20 @@
 #define UART_PADDR	MX3x_UART1_BASE_ADDR
 #endif
 
-#ifdef CONFIG_ARCH_MX5
+#ifdef CONFIG_ARCH_MX51
 #ifdef UART_PADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
 #define UART_PADDR	MX51_UART1_BASE_ADDR
 #endif
 
+#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
+#ifdef UART_PADDR
+#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
+#endif
+#define UART_PADDR	MX53_UART1_BASE_ADDR
+#endif
+
 #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
 
 		.macro	addruart, rp, rv
-- 
1.7.0.4

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Amit Kucheria]

On 11 Jul 09, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Amit Kucheria <amit.kucheria@canonical.com>

but I think we should switch to runtime detection of the machine (machine
ID?) and then set the port address.

See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.

> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..3919ad4 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -37,13 +37,20 @@
>  #define UART_PADDR	MX3x_UART1_BASE_ADDR
>  #endif
>  
> -#ifdef CONFIG_ARCH_MX5
> +#ifdef CONFIG_ARCH_MX51
>  #ifdef UART_PADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
>  #endif
>  
> +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> +#ifdef UART_PADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#define UART_PADDR	MX53_UART1_BASE_ADDR
> +#endif
> +
>  #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
>  
>  		.macro	addruart, rp, rv
> -- 
> 1.7.0.4
> 

-- 
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || amit.kucheria at canonical.com
----------------------------------------------------------------------

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Uwe Kleine-König]

On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> On 11 Jul 09, Troy Kisky wrote:
> > The i.MX51 UART_PADDR value does not work for MX50/53.
> > 
> > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> 
> but I think we should switch to runtime detection of the machine (machine
> ID?) and then set the port address.
> 
> See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
mach/debug-macro.S and mach/uncompress.h are two different things. And
the uncompress stuff is dynamic for mxc AFAIK.

> > ---
> >  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > index 8e8d175..3919ad4 100644
> > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > @@ -37,13 +37,20 @@
> >  #define UART_PADDR	MX3x_UART1_BASE_ADDR
> >  #endif
> >  
> > -#ifdef CONFIG_ARCH_MX5
> > +#ifdef CONFIG_ARCH_MX51
> >  #ifdef UART_PADDR
> >  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> >  #endif
> >  #define UART_PADDR	MX51_UART1_BASE_ADDR
> >  #endif
> >  
> > +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> > +#ifdef UART_PADDR
> > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > +#endif
> > +#define UART_PADDR	MX53_UART1_BASE_ADDR
> > +#endif
> > +
Hm, even though it's obvious, maybe add a comment that
MX50_UART1_BASE_ADDR is identical to MX53_UART1_BASE_ADDR?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Amit Kucheria]

On 11 Jul 11, Uwe Kleine-K?nig wrote:
> On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> > On 11 Jul 09, Troy Kisky wrote:
> > > The i.MX51 UART_PADDR value does not work for MX50/53.
> > > 
> > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > 
> > Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> > 
> > but I think we should switch to runtime detection of the machine (machine
> > ID?) and then set the port address.
> > 
> > See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
> mach/debug-macro.S and mach/uncompress.h are two different things. And
> the uncompress stuff is dynamic for mxc AFAIK.
 
Indeed they're different. But it seems to me that
arch/arm/mach-omap2/include/mach/debug-macro.S and arch/arm/plat-omap/include/plat/uncompress.h
show clearly how we can deal with the serial port addresses (for DEBUG_LL)
dynamically. No?

> > > ---
> > >  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
> > >  1 files changed, 8 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > index 8e8d175..3919ad4 100644
> > > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > @@ -37,13 +37,20 @@
> > >  #define UART_PADDR	MX3x_UART1_BASE_ADDR
> > >  #endif
> > >  
> > > -#ifdef CONFIG_ARCH_MX5
> > > +#ifdef CONFIG_ARCH_MX51
> > >  #ifdef UART_PADDR
> > >  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > >  #endif
> > >  #define UART_PADDR	MX51_UART1_BASE_ADDR
> > >  #endif
> > >  
> > > +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> > > +#ifdef UART_PADDR
> > > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > > +#endif
> > > +#define UART_PADDR	MX53_UART1_BASE_ADDR
> > > +#endif
> > > +
> Hm, even though it's obvious, maybe add a comment that
> MX50_UART1_BASE_ADDR is identical to MX53_UART1_BASE_ADDR?
> 

-- 
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || amit.kucheria at canonical.com
----------------------------------------------------------------------

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Uwe Kleine-König]

On Mon, Jul 11, 2011 at 01:08:38PM +0300, Amit Kucheria wrote:
> On 11 Jul 11, Uwe Kleine-K?nig wrote:
> > On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> > > On 11 Jul 09, Troy Kisky wrote:
> > > > The i.MX51 UART_PADDR value does not work for MX50/53.
> > > > 
> > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > > 
> > > Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> > > 
> > > but I think we should switch to runtime detection of the machine (machine
> > > ID?) and then set the port address.
> > > 
> > > See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
> > mach/debug-macro.S and mach/uncompress.h are two different things. And
> > the uncompress stuff is dynamic for mxc AFAIK.
>  
> Indeed they're different. But it seems to me that
> arch/arm/mach-omap2/include/mach/debug-macro.S and arch/arm/plat-omap/include/plat/uncompress.h
> show clearly how we can deal with the serial port addresses (for DEBUG_LL)
> dynamically. No?
Looking at arch/arm/mach-omap2/include/mach/debug-macro.S it doesn't
look easy. And considering that debug-macro.S is only needed for early
board bringup, it's totally OK to have hardcoded values for only a
single SOC at a time in it. And I really like to keep this simple as it
is the first working debug mechanism during boot.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Sascha Hauer]

Troy,

On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
instead of CONFIG_ARCH_*.

Sascha

> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..3919ad4 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -37,13 +37,20 @@
>  #define UART_PADDR	MX3x_UART1_BASE_ADDR
>  #endif
>  
> -#ifdef CONFIG_ARCH_MX5
> +#ifdef CONFIG_ARCH_MX51
>  #ifdef UART_PADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
>  #endif
>  
> +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> +#ifdef UART_PADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#define UART_PADDR	MX53_UART1_BASE_ADDR
> +#endif
> +
>  #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
>  
>  		.macro	addruart, rp, rv
> -- 
> 1.7.0.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Troy Kisky]

On 7/12/2011 6:32 AM, Sascha Hauer wrote:
> Troy,
> 
> On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
>> The i.MX51 UART_PADDR value does not work for MX50/53.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
> instead of CONFIG_ARCH_*.
> 
> Sascha
> 
>> ---

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Sascha Hauer]

On Tue, Jul 12, 2011 at 10:04:39AM -0700, Troy Kisky wrote:
> On 7/12/2011 6:32 AM, Sascha Hauer wrote:
> > Troy,
> > 
> > On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
> >> The i.MX51 UART_PADDR value does not work for MX50/53.
> >>
> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > 
> > Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
> > instead of CONFIG_ARCH_*.
> > 
> > Sascha
> > 
> >> ---
> 
> From arch/arm/mach-mx5/Kconfig
> 
> # ARCH_MX5/50/53 are left to mark places where prevent multi-soc in single
> # image. So for most time, SOC_IMX50/51/53 should be used.
> 
> 
> So, it would seem to me that the ARCH variant would be correct.

No, we want to get rid of CONFIG_ARCH_MX*. The debug macro stuff is a
bit special as we intentionally disable multi soc kernels for the low
level debug case.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Wolfram Sang]

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?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110720/3ce61987/attachment.sig>

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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
+
 #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
 
 		.macro	addruart, rp, rv
-- 
1.7.4.1

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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.

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Wolfram Sang]

> > 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.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110720/72b15bdd/attachment.sig>

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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.

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Wolfram Sang]

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?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110720/fa5ceddc/attachment.sig>

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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.

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Troy Kisky]

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

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Wolfram Sang]

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 :)

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110720/063da325/attachment.sig>

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Troy Kisky]

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

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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.

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Shawn Guo]

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? :)

-- 
Regards,
Shawn

[PATCH] arm/mxc: add the missing UART_PADDR for i.mx53

[Troy Kisky]

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 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Troy Kisky]

On 7/12/2011 6:34 AM, Sascha Hauer wrote:
> On Mon, Jul 11, 2011 at 07:40:44PM -0700, Troy Kisky wrote:
>> The i.MX51 UART_PADDR value does not work for MX50/53.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> Your first patch looked better...
> 
> Sascha

I agree, I was trying to address Uwe's complaint.

Troy

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Sascha Hauer]

On Mon, Jul 11, 2011 at 07:40:44PM -0700, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Your first patch looked better...

Sascha

> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |   42 +++++++++++++++++++++-----
>  1 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..6b696cd 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -17,32 +17,58 @@
>  #endif
>  
>  #ifdef CONFIG_ARCH_MX25
> -#ifdef UART_PADDR
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX25_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX25_UART1_BASE_ADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
> -#define UART_PADDR	MX25_UART1_BASE_ADDR
>  #endif
>  
>  #ifdef CONFIG_ARCH_MX2
> -#ifdef UART_PADDR
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX2x_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX2x_UART1_BASE_ADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
> -#define UART_PADDR	MX2x_UART1_BASE_ADDR
>  #endif
>  
>  #ifdef CONFIG_ARCH_MX3
> -#ifdef UART_PADDR
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX3x_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX3x_UART1_BASE_ADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
> -#define UART_PADDR	MX3x_UART1_BASE_ADDR
>  #endif
>  
> -#ifdef CONFIG_ARCH_MX5
> -#ifdef UART_PADDR
> +#ifdef CONFIG_ARCH_MX50
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX50_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX50_UART1_BASE_ADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
> +#endif
> +
> +#ifdef CONFIG_ARCH_MX51
> +#ifndef UART_PADDR
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
>  #endif
> +#if UART_PADDR != MX51_UART1_BASE_ADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#endif
> +
> +#ifdef CONFIG_ARCH_MX53
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX53_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX53_UART1_BASE_ADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#endif
>  
>  #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
>  
> -- 
> 1.7.0.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Uwe Kleine-König]

On Mon, Jul 11, 2011 at 07:40:44PM -0700, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |   42 +++++++++++++++++++++-----
>  1 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..6b696cd 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -17,32 +17,58 @@
>  #endif
>  
>  #ifdef CONFIG_ARCH_MX25
> -#ifdef UART_PADDR
> +#ifndef UART_PADDR
> +#define UART_PADDR	MX25_UART1_BASE_ADDR
> +#endif
> +#if UART_PADDR != MX25_UART1_BASE_ADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
> -#define UART_PADDR	MX25_UART1_BASE_ADDR
>  #endif
I'm not sure it's worth the effort to do this. Still you should at least
note it in the change log or better move it to a separate patch.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

[Troy Kisky]

The i.MX51 UART_PADDR value does not work for MX50/53.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/plat-mxc/include/mach/debug-macro.S |   42 +++++++++++++++++++++-----
 1 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
index 8e8d175..6b696cd 100644
--- a/arch/arm/plat-mxc/include/mach/debug-macro.S
+++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
@@ -17,32 +17,58 @@
 #endif
 
 #ifdef CONFIG_ARCH_MX25
-#ifdef UART_PADDR
+#ifndef UART_PADDR
+#define UART_PADDR	MX25_UART1_BASE_ADDR
+#endif
+#if UART_PADDR != MX25_UART1_BASE_ADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
-#define UART_PADDR	MX25_UART1_BASE_ADDR
 #endif
 
 #ifdef CONFIG_ARCH_MX2
-#ifdef UART_PADDR
+#ifndef UART_PADDR
+#define UART_PADDR	MX2x_UART1_BASE_ADDR
+#endif
+#if UART_PADDR != MX2x_UART1_BASE_ADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
-#define UART_PADDR	MX2x_UART1_BASE_ADDR
 #endif
 
 #ifdef CONFIG_ARCH_MX3
-#ifdef UART_PADDR
+#ifndef UART_PADDR
+#define UART_PADDR	MX3x_UART1_BASE_ADDR
+#endif
+#if UART_PADDR != MX3x_UART1_BASE_ADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
-#define UART_PADDR	MX3x_UART1_BASE_ADDR
 #endif
 
-#ifdef CONFIG_ARCH_MX5
-#ifdef UART_PADDR
+#ifdef CONFIG_ARCH_MX50
+#ifndef UART_PADDR
+#define UART_PADDR	MX50_UART1_BASE_ADDR
+#endif
+#if UART_PADDR != MX50_UART1_BASE_ADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
+#endif
+
+#ifdef CONFIG_ARCH_MX51
+#ifndef UART_PADDR
 #define UART_PADDR	MX51_UART1_BASE_ADDR
 #endif
+#if UART_PADDR != MX51_UART1_BASE_ADDR
+#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
+#endif
+#endif
+
+#ifdef CONFIG_ARCH_MX53
+#ifndef UART_PADDR
+#define UART_PADDR	MX53_UART1_BASE_ADDR
+#endif
+#if UART_PADDR != MX53_UART1_BASE_ADDR
+#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
+#endif
+#endif
 
 #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
 
-- 
1.7.0.4