Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Dear Premi,
Sanjeev Premi wrote:
> The function display_board_info() displays the silicon
> revision as 2 - based on the return value from get_cpu_rev().
>
> This is incorrect as the current Si version is 3.1
Thanks for the patch and fixing this!
> This patch displays the correct version; but does not
> change get_cpu_rev() to minimize the code impact.
I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?
A quick grep resulted in 5 (?) locations which might be affected:
./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() == CPU_3430_ES2) {
./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() == CPU_3430_ES2) {
./cpu/arm_cortexa8/omap3/clock.c:173: sil_index = get_cpu_rev() - 1;
./cpu/arm_cortexa8/omap3/sys_info.c:144:if (get_cpu_rev() ==
CPU_3430_ES2)
./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
get_cpu_rev());
If we extend the existing macros
#define CPU_3430_ES11
#define CPU_3430_ES22
to e.g.
#define CPU_3430_ES10 1
#define CPU_3430_ES20 2
#define CPU_3430_ES21 3
#define CPU_3430_ES30 4
#define CPU_3430_ES31 5
then the three
== CPU_3430_ES2
will simply become
>= CPU_3430_ES20
The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
Regarding the ASCII strings: With the numbers get_cpu_rev() returns
we then could index a const struct with the ASCII strings for the
revision print. E.g.
printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
What do you think?
> Signed-off-by: Sanjeev Premi
> ---
> cpu/arm_cortexa8/omap3/sys_info.c | 37
> +++--
> 1 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
> b/cpu/arm_cortexa8/omap3/sys_info.c
> index b385b91..8c6a4d6 100644
> --- a/cpu/arm_cortexa8/omap3/sys_info.c
> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t
> *)GPMC_CONFIG_CS0_BASE;
> static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>
> +static char omap_revision[8] = "";
> +
> /*
> * dieid_num_r(void) - read and set die ID
> */
> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>
> }
>
> +/**
> + * Converts cpu revision into a string
> + */
> +void set_omap_revision(void)
> +{
> + u32 idcode;
> + ctrl_id_t *id_base;
> + char *str_rev = &omap_revision[0];
> +
> + if (get_cpu_rev() == CPU_3430_ES1) {
> + strcat (str_rev, "ES1.0");
> + }
> + else {
> + id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> +
> + idcode = readl(&id_base->idcode);
> +
> + if (idcode == 0x1B7AE02F)
> + strcat (str_rev, "ES2.0");
> + else if (idcode == 0x2B7AE02F)
> + strcat (str_rev, "ES2.1");
> + else if (idcode == 0x3B7AE02F)
> + strcat (str_rev, "ES3.0");
> + else if (idcode == 0x4B7AE02F)
It looks to me that only the highest nibble of idcode changes here?
Maybe we could better mask & shift it a little and create a nice macro
for it?
Best regards
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> -Original Message-
> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> Sent: Tuesday, April 21, 2009 10:26 PM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>
> Dear Premi,
>
> Sanjeev Premi wrote:
> > The function display_board_info() displays the silicon
> > revision as 2 - based on the return value from get_cpu_rev().
> >
> > This is incorrect as the current Si version is 3.1
>
> Thanks for the patch and fixing this!
>
> > This patch displays the correct version; but does not
> > change get_cpu_rev() to minimize the code impact.
>
> I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?
Yes. This is what I started with; but then this is where I felt that
fix may run 'deeper"
u32 get_board_type(void)
{
if (get_cpu_rev() == CPU_3430_ES2)
return sysinfo.board_type_v2;
else
return sysinfo.board_type_v1;
}
I couldn't figure out how this impacts boards other than the EVM.
>
> A quick grep resulted in 5 (?) locations which might be affected:
>
> ./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() == CPU_3430_ES2) {
>
> ./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() == CPU_3430_ES2) {
>
> ./cpu/arm_cortexa8/omap3/clock.c:173: sil_index =
> get_cpu_rev() - 1;
>
> ./cpu/arm_cortexa8/omap3/sys_info.c:144:if (get_cpu_rev() ==
> CPU_3430_ES2)
> ./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
> get_cpu_rev());
>
> If we extend the existing macros
>
> #define CPU_3430_ES1 1
> #define CPU_3430_ES2 2
>
> to e.g.
>
> #define CPU_3430_ES10 1
> #define CPU_3430_ES20 2
> #define CPU_3430_ES21 3
> #define CPU_3430_ES30 4
> #define CPU_3430_ES31 5
>
> then the three
>
> == CPU_3430_ES2
>
> will simply become
>
> >= CPU_3430_ES20
>
> The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
>
> Regarding the ASCII strings: With the numbers get_cpu_rev() returns
> we then could index a const struct with the ASCII strings for the
> revision print. E.g.
>
> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
>
> What do you think?
>
> > Signed-off-by: Sanjeev Premi
> > ---
> > cpu/arm_cortexa8/omap3/sys_info.c | 37
> +++--
> > 1 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
> b/cpu/arm_cortexa8/omap3/sys_info.c
> > index b385b91..8c6a4d6 100644
> > --- a/cpu/arm_cortexa8/omap3/sys_info.c
> > +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> > @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base =
> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
> > static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> > static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
> >
> > +static char omap_revision[8] = "";
> > +
> > /*
> > * dieid_num_r(void) - read and set die ID
> > */
> > @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
> >
> > }
> >
> > +/**
> > + * Converts cpu revision into a string
> > + */
> > +void set_omap_revision(void)
> > +{
> > + u32 idcode;
> > + ctrl_id_t *id_base;
> > + char *str_rev = &omap_revision[0];
> > +
> > + if (get_cpu_rev() == CPU_3430_ES1) {
> > + strcat (str_rev, "ES1.0");
> > + }
> > + else {
> > + id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> > +
> > + idcode = readl(&id_base->idcode);
> > +
> > + if (idcode == 0x1B7AE02F)
> > + strcat (str_rev, "ES2.0");
> > + else if (idcode == 0x2B7AE02F)
> > + strcat (str_rev, "ES2.1");
> > + else if (idcode == 0x3B7AE02F)
> > + strcat (str_rev, "ES3.0");
> > + else if (idcode == 0x4B7AE02F)
>
> It looks to me that only the highest nibble of idcode changes here?
> Maybe we could better mask & shift it a little and create a
> nice macro
> for it?
>
> Best regards
>
> Dirk
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Premi, Sanjeev wrote:
>> -Original Message-
>> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
>> Sent: Tuesday, April 21, 2009 10:26 PM
>> To: Premi, Sanjeev
>> Cc: u-boot@lists.denx.de
>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>
>> Dear Premi,
>>
>> Sanjeev Premi wrote:
>>> The function display_board_info() displays the silicon
>>> revision as 2 - based on the return value from get_cpu_rev().
>>>
>>> This is incorrect as the current Si version is 3.1
>> Thanks for the patch and fixing this!
>>
>>> This patch displays the correct version; but does not
>>> change get_cpu_rev() to minimize the code impact.
>> I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?
>
> Yes. This is what I started with; but then this is where I felt that
> fix may run 'deeper"
>
> u32 get_board_type(void)
> {
> if (get_cpu_rev() == CPU_3430_ES2)
> return sysinfo.board_type_v2;
> else
> return sysinfo.board_type_v1;
> }
>
> I couldn't figure out how this impacts boards other than the EVM.
Maybe I missed something, but independent of what this function does,
if we replace
if (get_cpu_rev() == CPU_3430_ES2)
with
if (get_cpu_rev() >= CPU_3430_ES20)
the functionality of this function (i.e. the value returned) wouldn't
change compared to what it actually returns?
Best regards
Dirk
>> A quick grep resulted in 5 (?) locations which might be affected:
>>
>> ./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() == CPU_3430_ES2) {
>>
>> ./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() == CPU_3430_ES2) {
>>
>> ./cpu/arm_cortexa8/omap3/clock.c:173: sil_index =
>> get_cpu_rev() - 1;
>>
>> ./cpu/arm_cortexa8/omap3/sys_info.c:144:if (get_cpu_rev() ==
>> CPU_3430_ES2)
>> ./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
>> get_cpu_rev());
>>
>> If we extend the existing macros
>>
>> #define CPU_3430_ES1 1
>> #define CPU_3430_ES2 2
>>
>> to e.g.
>>
>> #define CPU_3430_ES101
>> #define CPU_3430_ES202
>> #define CPU_3430_ES213
>> #define CPU_3430_ES304
>> #define CPU_3430_ES315
>>
>> then the three
>>
>> == CPU_3430_ES2
>>
>> will simply become
>>
>> >= CPU_3430_ES20
>>
>> The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
>>
>> Regarding the ASCII strings: With the numbers get_cpu_rev() returns
>> we then could index a const struct with the ASCII strings for the
>> revision print. E.g.
>>
>> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
>>
>> What do you think?
>>
>>> Signed-off-by: Sanjeev Premi
>>> ---
>>> cpu/arm_cortexa8/omap3/sys_info.c | 37
>> +++--
>>> 1 files changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
>> b/cpu/arm_cortexa8/omap3/sys_info.c
>>> index b385b91..8c6a4d6 100644
>>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
>>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
>>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base =
>> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>>> static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>>> static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>>
>>> +static char omap_revision[8] = "";
>>> +
>>> /*
>>> * dieid_num_r(void) - read and set die ID
>>> */
>>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>>>
>>> }
>>>
>>> +/**
>>> + * Converts cpu revision into a string
>>> + */
>>> +void set_omap_revision(void)
>>> +{
>>> + u32 idcode;
>>> + ctrl_id_t *id_base;
>>> + char *str_rev = &omap_revision[0];
>>> +
>>> + if (get_cpu_rev() == CPU_3430_ES1) {
>>> + strcat (str_rev, "ES1.0");
>>> + }
>>> + else {
>>> + id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
>>> +
>>> + idcode = readl(&id_base->idcode);
>>> +
>>> + if (idcode == 0x1B7AE02F)
>>> + strcat (str_rev, "ES2.0");
>>> + else if (idcode == 0x2B7AE02F)
>>> + strcat (str_rev, "ES2.1");
>>> + else if (idcode == 0x3B7AE02F)
>>> + strcat (str_rev, "ES3.0");
>>> + else if (idcode == 0x4B7AE02F)
>> It looks to me that only the highest nibble of idcode changes here?
>> Maybe we could better mask & shift it a little and create a
>> nice macro
>> for it?
>>
>> Best regards
>>
>> Dirk
>>
>>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> -Original Message-
> From: Premi, Sanjeev
> Sent: Tuesday, April 21, 2009 11:37 PM
> To: 'Dirk Behme'
> Cc: u-boot@lists.denx.de
> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>
>
> > -Original Message-
> > From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> > Sent: Tuesday, April 21, 2009 10:26 PM
> > To: Premi, Sanjeev
> > Cc: u-boot@lists.denx.de
> > Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> >
> > Dear Premi,
> >
> > Sanjeev Premi wrote:
> > > The function display_board_info() displays the silicon
> > > revision as 2 - based on the return value from get_cpu_rev().
> > >
> > > This is incorrect as the current Si version is 3.1
> >
> > Thanks for the patch and fixing this!
> >
> > > This patch displays the correct version; but does not
> > > change get_cpu_rev() to minimize the code impact.
> >
> > I wonder if it wouldn't be better (and cleaner) to fix
> get_cpu_rev()?
>
> Yes. This is what I started with; but then this is where I felt that
> fix may run 'deeper"
>
> u32 get_board_type(void)
> {
> if (get_cpu_rev() == CPU_3430_ES2)
> return sysinfo.board_type_v2;
> else
> return sysinfo.board_type_v1;
> }
>
...sorry, mail 'went' before I wanted to!
> I couldn't figure out how this impacts boards other than the EVM.
Though I admit not having much time looking for the impact. Beyond
this, I believe the fix could be straight forward.
> >
> > A quick grep resulted in 5 (?) locations which might be affected:
> >
> > ./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() ==
> CPU_3430_ES2) {
> >
> > ./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() ==
> CPU_3430_ES2) {
> >
> > ./cpu/arm_cortexa8/omap3/clock.c:173: sil_index =
> > get_cpu_rev() - 1;
> >
> > ./cpu/arm_cortexa8/omap3/sys_info.c:144:if
> (get_cpu_rev() ==
> > CPU_3430_ES2)
> > ./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
> > get_cpu_rev());
> >
> > If we extend the existing macros
> >
> > #define CPU_3430_ES11
> > #define CPU_3430_ES22
> >
> > to e.g.
> >
> > #define CPU_3430_ES10 1
> > #define CPU_3430_ES20 2
> > #define CPU_3430_ES21 3
> > #define CPU_3430_ES30 4
> > #define CPU_3430_ES31 5
> >
> > then the three
> >
> > == CPU_3430_ES2
> >
> > will simply become
> >
> > >= CPU_3430_ES20
There seems to be a slight differene between the silicon
revision between 34x and 35x for the highest nibble value
for early si revs - ES 1.0 and ES2.0.
> >
> > The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
> >
> > Regarding the ASCII strings: With the numbers get_cpu_rev()
> returns
> > we then could index a const struct with the ASCII strings for the
> > revision print. E.g.
> >
> > printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
> >
> > What do you think?
> >
> > > Signed-off-by: Sanjeev Premi
> > > ---
> > > cpu/arm_cortexa8/omap3/sys_info.c | 37
> > +++--
> > > 1 files changed, 35 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
> > b/cpu/arm_cortexa8/omap3/sys_info.c
> > > index b385b91..8c6a4d6 100644
> > > --- a/cpu/arm_cortexa8/omap3/sys_info.c
> > > +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> > > @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base =
> > (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
> > > static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> > > static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
> > >
> > > +static char omap_revision[8] = "";
> > > +
> > >
> /*
> > > * dieid_num_r(void) - read and set die ID
> > >
> */
> > > @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
> > >
> > > }
> > >
> > > +/**
> > > + * Converts cpu revision into a string
> > > + */
> > > +void set_omap_revision(void)
> > > +{
> > > + u32 idcode;
> > > + ctrl_id_t *id_base;
> > > + char *str_r
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Dear Premi,
Premi, Sanjeev wrote:
-Original Message-
From: Premi, Sanjeev
Sent: Tuesday, April 21, 2009 11:37 PM
To: 'Dirk Behme'
Cc: u-boot@lists.denx.de
Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
-Original Message-
From: Dirk Behme [mailto:dirk.be...@googlemail.com]
Sent: Tuesday, April 21, 2009 10:26 PM
To: Premi, Sanjeev
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Dear Premi,
Sanjeev Premi wrote:
The function display_board_info() displays the silicon
revision as 2 - based on the return value from get_cpu_rev().
This is incorrect as the current Si version is 3.1
Thanks for the patch and fixing this!
This patch displays the correct version; but does not
change get_cpu_rev() to minimize the code impact.
I wonder if it wouldn't be better (and cleaner) to fix
get_cpu_rev()?
Yes. This is what I started with; but then this is where I felt that
fix may run 'deeper"
u32 get_board_type(void)
{
if (get_cpu_rev() == CPU_3430_ES2)
return sysinfo.board_type_v2;
else
return sysinfo.board_type_v1;
}
...sorry, mail 'went' before I wanted to!
I couldn't figure out how this impacts boards other than the EVM.
Though I admit not having much time looking for the impact. Beyond
this, I believe the fix could be straight forward.
What's about something like in the attachment? Compile tested only. Do
you like to test it?
Best regards
Dirk
A quick grep resulted in 5 (?) locations which might be affected:
./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() ==
CPU_3430_ES2) {
./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() ==
CPU_3430_ES2) {
./cpu/arm_cortexa8/omap3/clock.c:173: sil_index =
get_cpu_rev() - 1;
./cpu/arm_cortexa8/omap3/sys_info.c:144:if
(get_cpu_rev() ==
CPU_3430_ES2)
./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
get_cpu_rev());
If we extend the existing macros
#define CPU_3430_ES11
#define CPU_3430_ES22
to e.g.
#define CPU_3430_ES10 1
#define CPU_3430_ES20 2
#define CPU_3430_ES21 3
#define CPU_3430_ES30 4
#define CPU_3430_ES31 5
then the three
== CPU_3430_ES2
will simply become
>= CPU_3430_ES20
There seems to be a slight differene between the silicon
revision between 34x and 35x for the highest nibble value
for early si revs - ES 1.0 and ES2.0.
The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
Regarding the ASCII strings: With the numbers get_cpu_rev()
returns
we then could index a const struct with the ASCII strings for the
revision print. E.g.
printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
What do you think?
Signed-off-by: Sanjeev Premi
---
cpu/arm_cortexa8/omap3/sys_info.c | 37
+++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
b/cpu/arm_cortexa8/omap3/sys_info.c
index b385b91..8c6a4d6 100644
--- a/cpu/arm_cortexa8/omap3/sys_info.c
+++ b/cpu/arm_cortexa8/omap3/sys_info.c
@@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base =
(gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
+static char omap_revision[8] = "";
+
/*
* dieid_num_r(void) - read and set die ID
*/
@@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
}
+/**
+ * Converts cpu revision into a string
+ */
+void set_omap_revision(void)
+{
+ u32 idcode;
+ ctrl_id_t *id_base;
+ char *str_rev = &omap_revision[0];
+
+ if (get_cpu_rev() == CPU_3430_ES1) {
+ strcat (str_rev, "ES1.0");
+ }
+ else {
+ id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
+
+ idcode = readl(&id_base->idcode);
+
+ if (idcode == 0x1B7AE02F)
+ strcat (str_rev, "ES2.0");
+ else if (idcode == 0x2B7AE02F)
+ strcat (str_rev, "ES2.1");
+ else if (idcode == 0x3B7AE02F)
+ strcat (str_rev, "ES3.0");
+ else if (idcode == 0x4B7AE02F)
It looks to me that only the highest nibble of idcode changes here?
Maybe we could better mask & shift it a little and create a
nice macro
for it?
It is already done in the kernel; but I am not sure if we could save
much - unless we use the index as you suggest above.
Best regards
Dirk
Signed-off-by: Dirk Behme
---
cpu/arm_cortexa8/cpu.c |4 ++--
cpu/arm_cortexa8/omap3/clock.c |5 +++--
cpu/arm_cortexa8/omap3/sys_info.c | 31 +++-
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> -Original Message-
> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> Sent: Wednesday, April 22, 2009 1:04 AM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>
> Dear Premi,
>
> Premi, Sanjeev wrote:
> >> -Original Message-
> >> From: Premi, Sanjeev
> >> Sent: Tuesday, April 21, 2009 11:37 PM
> >> To: 'Dirk Behme'
> >> Cc: u-boot@lists.denx.de
> >> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> >>
> >>
> >>> -Original Message-
> >>> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> >>> Sent: Tuesday, April 21, 2009 10:26 PM
> >>> To: Premi, Sanjeev
> >>> Cc: u-boot@lists.denx.de
> >>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct
> silicon revision
> >>>
> >>> Dear Premi,
> >>>
> >>> Sanjeev Premi wrote:
> >>>> The function display_board_info() displays the silicon
> >>>> revision as 2 - based on the return value from get_cpu_rev().
> >>>>
> >>>> This is incorrect as the current Si version is 3.1
> >>> Thanks for the patch and fixing this!
> >>>
> >>>> This patch displays the correct version; but does not
> >>>> change get_cpu_rev() to minimize the code impact.
> >>> I wonder if it wouldn't be better (and cleaner) to fix
> >> get_cpu_rev()?
> >>
> >> Yes. This is what I started with; but then this is where I
> felt that
> >> fix may run 'deeper"
> >>
> >> u32 get_board_type(void)
> >> {
> >>if (get_cpu_rev() == CPU_3430_ES2)
> >>return sysinfo.board_type_v2;
> >>else
> >>return sysinfo.board_type_v1;
> >> }
> >>
> >
> > ...sorry, mail 'went' before I wanted to!
> >
> >> I couldn't figure out how this impacts boards other than the EVM.
> >
> > Though I admit not having much time looking for the impact. Beyond
> > this, I believe the fix could be straight forward.
>
> What's about something like in the attachment? Compile tested
> only. Do
> you like to test it?
Sure. Will do it in the morning...
I did make few changes since your last mail as well.
Will post the updates later in the day.
~sanjeev
>
> Best regards
>
> Dirk
>
> >>> A quick grep resulted in 5 (?) locations which might be affected:
> >>>
> >>> ./cpu/arm_cortexa8/cpu.c:104: if (get_cpu_rev() ==
> >> CPU_3430_ES2) {
> >>> ./cpu/arm_cortexa8/cpu.c:134: if (get_cpu_rev() ==
> >> CPU_3430_ES2) {
> >>> ./cpu/arm_cortexa8/omap3/clock.c:173: sil_index =
> >>> get_cpu_rev() - 1;
> >>>
> >>> ./cpu/arm_cortexa8/omap3/sys_info.c:144:if
> >> (get_cpu_rev() ==
> >>> CPU_3430_ES2)
> >>> ./cpu/arm_cortexa8/omap3/sys_info.c:237: sec_s,
> >>> get_cpu_rev());
> >>>
> >>> If we extend the existing macros
> >>>
> >>> #define CPU_3430_ES1 1
> >>> #define CPU_3430_ES2 2
> >>>
> >>> to e.g.
> >>>
> >>> #define CPU_3430_ES10 1
> >>> #define CPU_3430_ES20 2
> >>> #define CPU_3430_ES21 3
> >>> #define CPU_3430_ES30 4
> >>> #define CPU_3430_ES31 5
> >>>
> >>> then the three
> >>>
> >>> == CPU_3430_ES2
> >>>
> >>> will simply become
> >>>
> >>> >= CPU_3430_ES20
> >
> > There seems to be a slight differene between the silicon
> > revision between 34x and 35x for the highest nibble value
> > for early si revs - ES 1.0 and ES2.0.
> >
> >>> The sil_index = get_cpu_rev() - 1; needs a deeper look, though.
> >>>
> >>> Regarding the ASCII strings: With the numbers get_cpu_rev()
> >> returns
> >>> we then could index a const struct with the ASCII strings for the
> >>> revision print. E.g.
> >>>
> >>> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
> >>>
> >>> What do you think?
> >>>
> >>>> Signed-off-by: Sanjeev Premi
> >>>> ---
> >>>> cpu/arm_cortexa8/om
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> -Original Message-
> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> Sent: Wednesday, April 22, 2009 1:04 AM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>
> Dear Premi,
>
> Premi, Sanjeev wrote:
> >> -Original Message-
> >> From: Premi, Sanjeev
> >> Sent: Tuesday, April 21, 2009 11:37 PM
> >> To: 'Dirk Behme'
> >> Cc: u-boot@lists.denx.de
> >> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> >>
> >>
> >>> -Original Message-
> >>> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> >>> Sent: Tuesday, April 21, 2009 10:26 PM
> >>> To: Premi, Sanjeev
> >>> Cc: u-boot@lists.denx.de
> >>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct
> silicon revision
> >>>
> >>> Dear Premi,
> >>>
> >>> Sanjeev Premi wrote:
> >>>> The function display_board_info() displays the silicon
> >>>> revision as 2 - based on the return value from get_cpu_rev().
> >>>>
> >>>> This is incorrect as the current Si version is 3.1
> >>> Thanks for the patch and fixing this!
> >>>
> >>>> This patch displays the correct version; but does not
> >>>> change get_cpu_rev() to minimize the code impact.
> >>> I wonder if it wouldn't be better (and cleaner) to fix
> >> get_cpu_rev()?
> >>
> >> Yes. This is what I started with; but then this is where I
> felt that
> >> fix may run 'deeper"
> >>
> >> u32 get_board_type(void)
> >> {
> >>if (get_cpu_rev() == CPU_3430_ES2)
> >>return sysinfo.board_type_v2;
> >>else
> >>return sysinfo.board_type_v1;
> >> }
> >>
> >
> > ...sorry, mail 'went' before I wanted to!
> >
> >> I couldn't figure out how this impacts boards other than the EVM.
> >
> > Though I admit not having much time looking for the impact. Beyond
> > this, I believe the fix could be straight forward.
>
> What's about something like in the attachment? Compile tested
> only. Do
> you like to test it?
Yes, this works on the EVM.
I did spend some more time & fouund that value from get_board_type is
ignored in the display_board_info().
I will submit a patch to remove this function if it is really not needed.
Best regards,
Sanjeev
>
> Best regards
>
> Dirk
>
[...snip...]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Premi, Sanjeev wrote:
-Original Message-
From: Dirk Behme [mailto:dirk.be...@googlemail.com]
Sent: Wednesday, April 22, 2009 1:04 AM
To: Premi, Sanjeev
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Dear Premi,
Premi, Sanjeev wrote:
-Original Message-
From: Premi, Sanjeev
Sent: Tuesday, April 21, 2009 11:37 PM
To: 'Dirk Behme'
Cc: u-boot@lists.denx.de
Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
-Original Message-
From: Dirk Behme [mailto:dirk.be...@googlemail.com]
Sent: Tuesday, April 21, 2009 10:26 PM
To: Premi, Sanjeev
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct
silicon revision
Dear Premi,
Sanjeev Premi wrote:
The function display_board_info() displays the silicon
revision as 2 - based on the return value from get_cpu_rev().
This is incorrect as the current Si version is 3.1
Thanks for the patch and fixing this!
This patch displays the correct version; but does not
change get_cpu_rev() to minimize the code impact.
I wonder if it wouldn't be better (and cleaner) to fix
get_cpu_rev()?
Yes. This is what I started with; but then this is where I
felt that
fix may run 'deeper"
u32 get_board_type(void)
{
if (get_cpu_rev() == CPU_3430_ES2)
return sysinfo.board_type_v2;
else
return sysinfo.board_type_v1;
}
...sorry, mail 'went' before I wanted to!
I couldn't figure out how this impacts boards other than the EVM.
Though I admit not having much time looking for the impact. Beyond
this, I believe the fix could be straight forward.
What's about something like in the attachment? Compile tested
only. Do
you like to test it?
Yes, this works on the EVM.
Great, thanks for testing!
I did spend some more time & fouund that value from get_board_type is
ignored in the display_board_info().
Hmm, yes, good catch.
I will submit a patch to remove this function if it is really not needed.
Ok.
Best regards
Dirk
Btw.: Updated patch in attachment, maybe it helps you. Tested on Beagle.
Signed-off-by: Dirk Behme
---
board/omap3/beagle/beagle.h|2 -
board/omap3/evm/evm.h |2 -
board/omap3/overo/overo.h |2 -
board/omap3/pandora/pandora.h |2 -
board/omap3/zoom1/zoom1.h |2 -
cpu/arm_cortexa8/cpu.c |4 +-
cpu/arm_cortexa8/omap3/board.c |5 ---
cpu/arm_cortexa8/omap3/clock.c |5 ++-
cpu/arm_cortexa8/omap3/sys_info.c | 48 +++--
include/asm-arm/arch-omap3/omap3.h | 18 +---
include/asm-arm/arch-omap3/sys_proto.h |5 ---
11 files changed, 38 insertions(+), 57 deletions(-)
Index: u-boot-main/cpu/arm_cortexa8/cpu.c
===
--- u-boot-main.orig/cpu/arm_cortexa8/cpu.c
+++ u-boot-main/cpu/arm_cortexa8/cpu.c
@@ -101,7 +101,7 @@ void l2cache_enable()
volatile unsigned int j;
/* ES2 onwards we can disable/enable L2 ourselves */
- if (get_cpu_rev() == CPU_3430_ES2) {
+ if (get_cpu_rev() >= CPU_3XX_ES20) {
__asm__ __volatile__("mrc p15, 0, %0, c1, c0, 1":"=r"(i));
__asm__ __volatile__("orr %0, %0, #0x2":"=r"(i));
__asm__ __volatile__("mcr p15, 0, %0, c1, c0, 1":"=r"(i));
@@ -131,7 +131,7 @@ void l2cache_disable()
volatile unsigned int j;
/* ES2 onwards we can disable/enable L2 ourselves */
- if (get_cpu_rev() == CPU_3430_ES2) {
+ if (get_cpu_rev() >= CPU_3XX_ES20) {
__asm__ __volatile__("mrc p15, 0, %0, c1, c0, 1":"=r"(i));
__asm__ __volatile__("bic %0, %0, #0x2":"=r"(i));
__asm__ __volatile__("mcr p15, 0, %0, c1, c0, 1":"=r"(i));
Index: u-boot-main/cpu/arm_cortexa8/omap3/clock.c
===
--- u-boot-main.orig/cpu/arm_cortexa8/omap3/clock.c
+++ u-boot-main/cpu/arm_cortexa8/omap3/clock.c
@@ -132,7 +132,7 @@ void prcm_init(void)
void (*f_lock_pll) (u32, u32, u32, u32);
int xip_safe, p0, p1, p2, p3;
u32 osc_clk = 0, sys_clkin_sel;
- u32 clk_index, sil_index;
+ u32 clk_index, sil_index = 0;
prm_t *prm_base = (prm_t *)PRM_BASE;
prcm_t *prcm_base = (prcm_t *)PRCM_BASE;
dpll_param *dpll_param_p;
@@ -170,7 +170,8 @@ void prcm_init(void)
* and sil_index will get the values for that SysClk for the
* appropriate silicon rev.
*/
- sil_index = get_cpu_rev() - 1;
+ if(get_cpu_rev())
+ sil_index = 1;
/* Unlock MPU DPLL (slows things down, and needed later) */
sr32(&prcm_base->clken_p
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
On 21:53 Tue 21 Apr , Sanjeev Premi wrote:
> The function display_board_info() displays the silicon
> revision as 2 - based on the return value from get_cpu_rev().
>
> This is incorrect as the current Si version is 3.1
>
> This patch displays the correct version; but does not
> change get_cpu_rev() to minimize the code impact.
>
> Signed-off-by: Sanjeev Premi
> ---
> cpu/arm_cortexa8/omap3/sys_info.c | 37
> +++--
> 1 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
> b/cpu/arm_cortexa8/omap3/sys_info.c
> index b385b91..8c6a4d6 100644
> --- a/cpu/arm_cortexa8/omap3/sys_info.c
> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t
> *)GPMC_CONFIG_CS0_BASE;
> static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>
> +static char omap_revision[8] = "";
why 8?
you only have 5 chars
so
static char omap_revision[6] = "";
will be enough
> +
> /*
> * dieid_num_r(void) - read and set die ID
> */
> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>
> }
>
why not this
in the cpu header
#define ES201
#define ES212
#define ES303
#define ES314
> +/**
> + * Converts cpu revision into a string
> + */
> +void set_omap_revision(void)
char* get_str_omap_revision(void)
{
u32 idcode;
ctrl_id_t *id_base;
if (omap_revision[0] != '\0')
return omap_revision;
if (get_cpu_rev() == CPU_3430_ES1) {
strcat (omap_revision, "ES1.0");
}
else {
id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
idcode = readl(&id_base->idcode) >> 28;
switch(idcode) {
case ES20:
strcat (omap_revision, "ES2.0");
break;
case ES21:
strcat (omap_revision, "ES2.1");
break;
case ES30:
strcat (omap_revision, "ES3.0");
break;
case ES31:
strcat (omap_revision, "ES3.1");
break;
default:
strcat (str_rev, "ES??");
}
return omap_revision;
}
Best Regards,
J.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
Dear Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:53 Tue 21 Apr , Sanjeev Premi wrote:
>> The function display_board_info() displays the silicon
>> revision as 2 - based on the return value from get_cpu_rev().
>>
>> This is incorrect as the current Si version is 3.1
>>
>> This patch displays the correct version; but does not
>> change get_cpu_rev() to minimize the code impact.
>>
>> Signed-off-by: Sanjeev Premi
>> ---
>> cpu/arm_cortexa8/omap3/sys_info.c | 37
>> +++--
>> 1 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c
>> b/cpu/arm_cortexa8/omap3/sys_info.c
>> index b385b91..8c6a4d6 100644
>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t
>> *)GPMC_CONFIG_CS0_BASE;
>> static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>> static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>
>> +static char omap_revision[8] = "";
> why 8?
> you only have 5 chars
> so
> static char omap_revision[6] = "";
> will be enough
>> +
>> /*
>> * dieid_num_r(void) - read and set die ID
>> */
>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>>
>> }
>>
> why not this
> in the cpu header
>
> #define ES20 1
> #define ES21 2
> #define ES30 3
> #define ES31 4
>> +/**
>> + * Converts cpu revision into a string
>> + */
>> +void set_omap_revision(void)
> char* get_str_omap_revision(void)
> {
> u32 idcode;
> ctrl_id_t *id_base;
>
> if (omap_revision[0] != '\0')
> return omap_revision;
>
> if (get_cpu_rev() == CPU_3430_ES1) {
> strcat (omap_revision, "ES1.0");
> }
> else {
> id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
>
> idcode = readl(&id_base->idcode) >> 28;
>
> switch(idcode) {
> case ES20:
> strcat (omap_revision, "ES2.0");
> break;
> case ES21:
> strcat (omap_revision, "ES2.1");
> break;
> case ES30:
> strcat (omap_revision, "ES3.0");
> break;
> case ES31:
> strcat (omap_revision, "ES3.1");
> break;
> default:
> strcat (str_rev, "ES??");
> }
> return omap_revision;
> }
Did you notice that we had some discussion about this and that Premi
will send an completely redone patch, soon?
http://lists.denx.de/pipermail/u-boot/2009-April/051186.html
http://lists.denx.de/pipermail/u-boot/2009-April/051187.html
http://lists.denx.de/pipermail/u-boot/2009-April/051191.html
http://lists.denx.de/pipermail/u-boot/2009-April/051189.html
http://lists.denx.de/pipermail/u-boot/2009-April/051193.html
http://lists.denx.de/pipermail/u-boot/2009-April/051194.html
http://lists.denx.de/pipermail/u-boot/2009-April/051228.html
http://lists.denx.de/pipermail/u-boot/2009-April/051241.html
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
