[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Khasim Syed Mohammed]

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Tom]

Khasim Syed Mohammed wrote:
> From 15fbe5ff9ee2fd2f8da4c16805d6c7ccf7244bae Mon Sep 17 00:00:00 2001
> From: Syed Mohammed Khasim <khasim@ti.com>
> Date: Fri, 8 Jan 2010 20:13:47 +0530
> Subject: [PATCH] OMAP3 Beagle Update revision detection
> 
<snip>

>  	MUX_BEAGLE();
> 
> -	if (beagle_revision_c) {
> +	if (beagle_revision != REVISION_AXBX)
>  		MUX_BEAGLE_C();
> -	}

Because MUX_BEAGLE_C is a multiline macro,
it would be good to keep the '{ }'

>  }
> diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
> index 7fe6275..b1720c9 100644
> --- a/board/ti/beagle/beagle.h
> +++ b/board/ti/beagle/beagle.h
> @@ -33,7 +33,11 @@ const omap3_sysinfo sysinfo = {
>  #endif
>  };
> 
> -#define BOARD_REVISION_MASK	(0x1 << 11)
> +/* BeagleBoard revisions */
> +#define REVISION_AXBX 	0x7
> +#define REVISION_CX 	0x6
> +#define REVISION_C4 	0x5
> +#define REVISION_D 	0x0
> 
ws
remove spaces at the end of var's in the last 4 lines


Tom

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Khasim Syed Mohammed]

Hi Tom,

On Sun, Jan 17, 2010 at 5:44 AM, Tom <Tom.Rix@windriver.com> wrote:
> Khasim Syed Mohammed wrote:
>>
>> From 15fbe5ff9ee2fd2f8da4c16805d6c7ccf7244bae Mon Sep 17 00:00:00 2001
>> From: Syed Mohammed Khasim <khasim@ti.com>
>> Date: Fri, 8 Jan 2010 20:13:47 +0530
>> Subject: [PATCH] OMAP3 Beagle Update revision detection
>>
> <snip>
>
>> ? ? ? ?MUX_BEAGLE();
>>
>> - ? ? ? if (beagle_revision_c) {
>> + ? ? ? if (beagle_revision != REVISION_AXBX)
>> ? ? ? ? ? ? ? ?MUX_BEAGLE_C();
>> - ? ? ? }
>
> Because MUX_BEAGLE_C is a multiline macro,
> it would be good to keep the '{ }'
>
I had this before, but it resulted in Checkpatch error, do you still
want me to keep { } ?,

Thanks for the review,

Khasim

>> ?}
>> diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
>> index 7fe6275..b1720c9 100644
>> --- a/board/ti/beagle/beagle.h
>> +++ b/board/ti/beagle/beagle.h
>> @@ -33,7 +33,11 @@ const omap3_sysinfo sysinfo = {
>> ?#endif
>> ?};
>>
>> -#define BOARD_REVISION_MASK ? ?(0x1 << 11)
>> +/* BeagleBoard revisions */
>> +#define REVISION_AXBX ?0x7
>> +#define REVISION_CX ? ?0x6
>> +#define REVISION_C4 ? ?0x5
>> +#define REVISION_D ? ? 0x0
>>
> ws
> remove spaces at the end of var's in the last 4 lines
>
>
> Tom
>

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Tom]

Khasim Syed Mohammed wrote:
> Hi Tom,
> 
> On Sun, Jan 17, 2010 at 5:44 AM, Tom <Tom.Rix@windriver.com> wrote:
>> Khasim Syed Mohammed wrote:
>>> From 15fbe5ff9ee2fd2f8da4c16805d6c7ccf7244bae Mon Sep 17 00:00:00 2001
>>> From: Syed Mohammed Khasim <khasim@ti.com>
>>> Date: Fri, 8 Jan 2010 20:13:47 +0530
>>> Subject: [PATCH] OMAP3 Beagle Update revision detection
>>>
>> <snip>
>>
>>>        MUX_BEAGLE();
>>>
>>> -       if (beagle_revision_c) {
>>> +       if (beagle_revision != REVISION_AXBX)
>>>                MUX_BEAGLE_C();
>>> -       }
>> Because MUX_BEAGLE_C is a multiline macro,
>> it would be good to keep the '{ }'
>>
> I had this before, but it resulted in Checkpatch error, do you still
> want me to keep { } ?,
> 

Yes.
Checkpatch is not smart enough to figure out if it is function or
a macro.

Because macros are better behaved in a '{ }', block I prefer that.

Tom

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Khasim Syed Mohammed]

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Tom]

Khasim Syed Mohammed wrote:
> From 2dce4d64339a26144eac2ed56eb664cc1ea527c7 Mon Sep 17 00:00:00 2001
> From: Syed Mohammed Khasim <khasim@ti.com>
> Date: Mon, 18 Jan 2010 18:03:42 +0530
> Subject: [PATCH] OMAP3 Beagle Update revision detection
> 
> New BeagleBoard revision C4 uses a new ID. Update revision detection.
> 

This patch set caused regressions in MAKEALL arm
These must be fixed.

omap3_beagle
cmd_i2c.c:109: warning: missing braces around initializer
cmd_i2c.c:109: warning: (near initialization for 'i2c_no_probes[0]')
beagle.c: In function 'misc_init_r':
beagle.c:143: warning: implicit declaration of function 'twl4030_pmrecv_vsel_cfg'
beagle.c:152: warning: implicit declaration of function 'prcm_config_720mhz'

The changes I asked from for 1/7 were done.
Tom

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Dirk Behme]

On 24.01.2010 01:03, Tom wrote:
> Khasim Syed Mohammed wrote:
>>  From 2dce4d64339a26144eac2ed56eb664cc1ea527c7 Mon Sep 17 00:00:00 2001

Khasim: The 'From' line of your patch starts with an additional '>'

 >From 2dce4d64339a2 ...

Could you check where this comes from? Maybe this confuses the mailing 
list archive

http://lists.denx.de/pipermail/u-boot/2010-January/066629.html

?

And could you try to add a 'vX' version to the subject of your 
patches? E.g.

[PATCH 1/7 v4] OMAP3 Bea ....

This would really help to identify the patch versions.

>> From: Syed Mohammed Khasim<khasim@ti.com>
>> Date: Mon, 18 Jan 2010 18:03:42 +0530
>> Subject: [PATCH] OMAP3 Beagle Update revision detection
>>
>> New BeagleBoard revision C4 uses a new ID. Update revision detection.
>>
>
> This patch set caused regressions in MAKEALL arm

Tom: On recent master a84b655fab2bc021fdafa91d1196cf826474cdd8 this 
patch seems to compile fine (?)

Best regards

Dirk

> These must be fixed.
>
> omap3_beagle
> cmd_i2c.c:109: warning: missing braces around initializer
> cmd_i2c.c:109: warning: (near initialization for 'i2c_no_probes[0]')
> beagle.c: In function 'misc_init_r':
> beagle.c:143: warning: implicit declaration of function 'twl4030_pmrecv_vsel_cfg'
> beagle.c:152: warning: implicit declaration of function 'prcm_config_720mhz'
>
> The changes I asked from for 1/7 were done.
> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Wolfgang Denk]

Dear Dirk Behme,

In message <4B5C0C00.9070707@googlemail.com> you wrote:
>
> Khasim: The 'From' line of your patch starts with an additional '>'
> 
>  >From 2dce4d64339a2 ...
> 
> Could you check where this comes from? Maybe this confuses the mailing 
> list archive

This is pretty noremal. It should not affect anything. It is actually
needed for all MUAs who store their messages in mailbox format.


Best regards,

Wolfgang Denk

-- 
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 must follow the people.  Am I not their leader? - Benjamin Disraeli

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Dirk Behme]

On 24.01.2010 15:26, Wolfgang Denk wrote:
> Dear Dirk Behme,
>
> In message<4B5C0C00.9070707@googlemail.com>  you wrote:
>>
>> Khasim: The 'From' line of your patch starts with an additional '>'
>>
>>   > From 2dce4d64339a2 ...
>>
>> Could you check where this comes from? Maybe this confuses the mailing
>> list archive
>
> This is pretty noremal. It should not affect anything. It is actually
> needed for all MUAs who store their messages in mailbox format.

Ok.

Any other idea what causes the broken ml archive

http://lists.denx.de/pipermail/u-boot/2010-January/066629.html

then?

Best regards

Dirk

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Wolfgang Denk]

Dear Dirk Behme,

In message <4B5C9C8E.9060401@googlemail.com> you wrote:
>
> Any other idea what causes the broken ml archive
> 
> http://lists.denx.de/pipermail/u-boot/2010-January/066629.html
> 
> then?

I have no idea. The mbox entry looks ok, and also the article looks OK
on gmane.

Best regards,

Wolfgang Denk

-- 
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
If there are self-made purgatories, then we all have to live in them.
	-- Spock, "This Side of Paradise", stardate 3417.7

[U-Boot] TI:OMAP: [PATCH 1/7] OMAP3 Beagle Update revision detection

[Dirk Behme]

On 24.01.2010 01:03, Tom wrote:
> Khasim Syed Mohammed wrote:
>>  From 2dce4d64339a26144eac2ed56eb664cc1ea527c7 Mon Sep 17 00:00:00 2001
>> From: Syed Mohammed Khasim<khasim@ti.com>
>> Date: Mon, 18 Jan 2010 18:03:42 +0530
>> Subject: [PATCH] OMAP3 Beagle Update revision detection
>>
>> New BeagleBoard revision C4 uses a new ID. Update revision detection.
>>
>
> This patch set caused regressions in MAKEALL arm
> These must be fixed.
>
> omap3_beagle
> cmd_i2c.c:109: warning: missing braces around initializer
> cmd_i2c.c:109: warning: (near initialization for 'i2c_no_probes[0]')
> beagle.c: In function 'misc_init_r':
> beagle.c:143: warning: implicit declaration of function 'twl4030_pmrecv_vsel_cfg'
> beagle.c:152: warning: implicit declaration of function 'prcm_config_720mhz'

With recent U-Boot main git head 
"a1b322a98319de568b3bc38029ef7e123643c1e8" I don't see this:

 > ./MAKEALL omap3_beagle
Configuring for omap3_beagle board...
    text    data     bss     dec     hex filename
  184267    9764  202428  396459   60cab ./u-boot

--------------------- SUMMARY ----------------------------
Boards compiled: 1
---------------------------------------------------------

Is this already gone? Or how to reproduce it?

Dirk