Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
Dear Kumar Gala, In message <2cf0740e-8067-456c-b3aa-1d8ce3a76...@kernel.crashing.org> you wrote: > > > This loop is similar to what nand_spl/nand_boot.c is using. It's ugly, but > > the goal here is small code rather than cleanliness. Is the timebase > > running at this point? How much code is required to get the timebase > > frequency? > > TB isn't running so you have to turn it on in the SoC (so a few CCSRBAR > read/writes), than you have to calculate freq on some boards its a #define > constant, on other its calculated reading I2C which would add a bunch of code > for accessing I2C. I'm pret > ty sure we aren't going to be able to do that in 4k. No matter what exactly you do, the loop we have now cannot be used as it is completley dependent on the tool chain if there is any delay at all, and it's pretty much nondeterministic how long it will be using different tool chains. This code is simply broken and needs fixing. 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: w...@denx.de The human mind ordinarily operates at only ten percent of its capaci- ty - the rest is overhead for the operating system. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On May 4, 2011, at 1:02 PM, Scott Wood wrote:
> On Wed, 4 May 2011 12:34:20 -0500
> Kumar Gala wrote:
>
>>
>> On May 4, 2011, at 12:31 PM, Haiying Wang wrote:
>>
>>> On Wed, 2011-05-04 at 22:53 +0530, Poonam Aggrwal wrote:
+sinclude $(obj).depend
+
+#
diff --git a/nand_spl/board/freescale/p1010rdb/nand_boot.c
b/nand_spl/board/freescale/p1010rdb/nand_boot.c
new file mode 100644
index 000..f0de279
--- /dev/null
+++ b/nand_spl/board/freescale/p1010rdb/nand_boot.c
@@ -0,0 +1,119 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j <
1; j++); }
>>> There were many comments on this udelay before, we should not use this
>>> define, but use the udelay() which u-boot provides.
>>>
>>
>> Is there a udelay that is defined for the nand_spl build? The problem is
>> doing proper time based delay in nand_spl would require a lot more code.
>
> This loop is similar to what nand_spl/nand_boot.c is using. It's ugly, but
> the goal here is small code rather than cleanliness. Is the timebase
> running at this point? How much code is required to get the timebase
> frequency?
TB isn't running so you have to turn it on in the SoC (so a few CCSRBAR
read/writes), than you have to calculate freq on some boards its a #define
constant, on other its calculated reading I2C which would add a bunch of code
for accessing I2C. I'm pretty sure we aren't going to be able to do that in 4k.
- k
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On Wed, 4 May 2011 14:15:58 -0500
Andy Fleming wrote:
> >> >> +
> >> >> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j <
> >> >> 1; j++); }
> >> > There were many comments on this udelay before, we should not use this
> >> > define, but use the udelay() which u-boot provides.
> >> >
> >>
> >> Is there a udelay that is defined for the nand_spl build? The problem is
> >> doing proper time based delay in nand_spl would require a lot more code.
> >
> > This loop is similar to what nand_spl/nand_boot.c is using. It's ugly, but
> > the goal here is small code rather than cleanliness. Is the timebase
> > running at this point? How much code is required to get the timebase
> > frequency?
>
>
> Is it possible to compromise and not call it udelay? udelay(x) is
> supposed to delay for x microseconds, but clearly this delays for
> x*1 iterations through a loop. Also, are we sure that such a macro
> works at all? It looks like the sort of thing compilers optimize away
> all the time.
I think it used to special-case such empty-body loops, but apparently not
anymore. I did some test-compiles and such constructs are optimized away.
-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
>> >> +
>> >> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j <
>> >> 1; j++); }
>> > There were many comments on this udelay before, we should not use this
>> > define, but use the udelay() which u-boot provides.
>> >
>>
>> Is there a udelay that is defined for the nand_spl build? The problem is
>> doing proper time based delay in nand_spl would require a lot more code.
>
> This loop is similar to what nand_spl/nand_boot.c is using. It's ugly, but
> the goal here is small code rather than cleanliness. Is the timebase
> running at this point? How much code is required to get the timebase
> frequency?
Is it possible to compromise and not call it udelay? udelay(x) is
supposed to delay for x microseconds, but clearly this delays for
x*1 iterations through a loop. Also, are we sure that such a macro
works at all? It looks like the sort of thing compilers optimize away
all the time.
Andy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On Wed, 4 May 2011 12:34:20 -0500
Kumar Gala wrote:
>
> On May 4, 2011, at 12:31 PM, Haiying Wang wrote:
>
> > On Wed, 2011-05-04 at 22:53 +0530, Poonam Aggrwal wrote:
> >> +sinclude $(obj).depend
> >> +
> >> +#
> >> diff --git a/nand_spl/board/freescale/p1010rdb/nand_boot.c
> >> b/nand_spl/board/freescale/p1010rdb/nand_boot.c
> >> new file mode 100644
> >> index 000..f0de279
> >> --- /dev/null
> >> +++ b/nand_spl/board/freescale/p1010rdb/nand_boot.c
> >> @@ -0,0 +1,119 @@
> >> +/*
> >> + * Copyright 2011 Freescale Semiconductor, Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation; either version 2 of
> >> + * the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + *
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> >> + * MA 02111-1307 USA
> >> + *
> >> + */
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j <
> >> 1; j++); }
> > There were many comments on this udelay before, we should not use this
> > define, but use the udelay() which u-boot provides.
> >
>
> Is there a udelay that is defined for the nand_spl build? The problem is
> doing proper time based delay in nand_spl would require a lot more code.
This loop is similar to what nand_spl/nand_boot.c is using. It's ugly, but
the goal here is small code rather than cleanliness. Is the timebase
running at this point? How much code is required to get the timebase
frequency?
-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On Wed, 2011-05-04 at 12:34 -0500, Kumar Gala wrote:
> >> +
> >> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j <
> >> 1; j++); }
> > There were many comments on this udelay before, we should not use this
> > define, but use the udelay() which u-boot provides.
> >
>
> Is there a udelay that is defined for the nand_spl build? The problem is
> doing proper time based delay in nand_spl would require a lot more code.
>
No special udelay defined for nand_spl. I only saw the comments on not
to define a extra udelay like that. I understood the reason but that was
one of the reasons for TPL support.
Haiying
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On May 4, 2011, at 12:31 PM, Haiying Wang wrote:
> On Wed, 2011-05-04 at 22:53 +0530, Poonam Aggrwal wrote:
>> +sinclude $(obj).depend
>> +
>> +#
>> diff --git a/nand_spl/board/freescale/p1010rdb/nand_boot.c
>> b/nand_spl/board/freescale/p1010rdb/nand_boot.c
>> new file mode 100644
>> index 000..f0de279
>> --- /dev/null
>> +++ b/nand_spl/board/freescale/p1010rdb/nand_boot.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * Copyright 2011 Freescale Semiconductor, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + *
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + *
>> + */
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j < 1;
>> j++); }
> There were many comments on this udelay before, we should not use this
> define, but use the udelay() which u-boot provides.
>
Is there a udelay that is defined for the nand_spl build? The problem is doing
proper time based delay in nand_spl would require a lot more code.
- k
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] powerpc/85xx: Add basic support for P1010RDB
On Wed, 2011-05-04 at 22:53 +0530, Poonam Aggrwal wrote:
> +sinclude $(obj).depend
> +
> +#
> diff --git a/nand_spl/board/freescale/p1010rdb/nand_boot.c
> b/nand_spl/board/freescale/p1010rdb/nand_boot.c
> new file mode 100644
> index 000..f0de279
> --- /dev/null
> +++ b/nand_spl/board/freescale/p1010rdb/nand_boot.c
> @@ -0,0 +1,119 @@
> +/*
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + *
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define udelay(x) {int i, j; for (i = 0; i < x; i++) for (j = 0; j < 1;
> j++); }
There were many comments on this udelay before, we should not use this
define, but use the udelay() which u-boot provides.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
