Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
Hi, Stefan Stefan Roese wrote: > Felix, > > On Wednesday 15 July 2009 11:24:07 Felix Radensky wrote: > >> I think my first patch that open-coded part of ft_board_setup() related >> to EBC ranges >> was correct and tested on real hardware. I admit that I didn't test the >> second version >> of the patch, as I didn't have the hardware at hand. >> >> I'll submit a fix soon, but I still don't have hardware for testing, so >> I'd appreciate your >> help with that. >> > > Any news on this issue? Would be great if you could find the time to submit a > patch for this. Dirk (?) and I will do the testing. > > The problem with approach you suggest (copy ranges array, modify first entry, rewrite ranges array), is that ranges property doesn't cover all chip selects, only populated ones, and NOR is not always on CS0 (in case of boot from NAND). So you don't really know which entry should be modified. Do you have any other ideas how to do this elegantly, without copying code from generic ft_board_setup() ? Felix. Felix. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
Felix, On Wednesday 15 July 2009 11:24:07 Felix Radensky wrote: > I think my first patch that open-coded part of ft_board_setup() related > to EBC ranges > was correct and tested on real hardware. I admit that I didn't test the > second version > of the patch, as I didn't have the hardware at hand. > > I'll submit a fix soon, but I still don't have hardware for testing, so > I'd appreciate your > help with that. Any news on this issue? Would be great if you could find the time to submit a patch for this. Dirk (?) and I will do the testing. Thanks. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
> >> __ft_board_setup() sets the property "ranges", with an entry for > >> every member of EBC_NUM_BANKS. > >> Then the so called "fixup" happens and the property > "ranges" is set > >> again, this time with only one entry (for the nor flash). > All other > >> entries are lost. > >> > > > > I see. I didn't spot this problem in the original patch. > Thanks for catching. > > > > It shouldn't be too hard to fix this problem by reading the > complete > > ranges array back and patching only the first entry and > re-writing ranges again. > > > > > I think my first patch that open-coded part of > ft_board_setup() related to EBC ranges was correct and tested > on real hardware. I admit that I didn't test the second > version of the patch, as I didn't have the hardware at hand. > > I'll submit a fix soon, but I still don't have hardware for > testing, so I'd appreciate your help with that. Hmm, maybe something generic in fdt_support.c would be nice, like fdt_find_and_modify_prop(void *fdt, const char *node, const char *prop, const void *val, int len, int pos, int create) Cheers Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
Hi,
Stefan Roese wrote:
> On Wednesday 15 July 2009 09:28:58 Eibach, Dirk wrote:
>
>>> Please check the latest version of this code in
>>> canyonlands.c. It's now using the 4xx common
>>> __ft_board_setup() function. Please move to this version as well.
>>>
>> This code in canyonlands.c does not work for me:
>>
>> void ft_board_setup(void *blob, bd_t *bd)
>> {
>> u32 val[4];
>> int rc;
>>
>> __ft_board_setup(blob, bd);
>>
>> /* Fixup NOR mapping */
>> val[0] = CONFIG_SYS_NOR_CS; /* chip select number */
>> val[1] = 0; /* always 0 */
>> val[2] = CONFIG_SYS_FLASH_BASE_PHYS_L; /* we fixed up
>> this address */
>> val[3] = gd->bd->bi_flashsize;
>> rc = fdt_find_and_setprop(blob, "/plb/opb/ebc", "ranges",
>>val, sizeof(val), 1);
>>
>> __ft_board_setup() sets the property "ranges", with an entry for every
>> member of EBC_NUM_BANKS.
>> Then the so called "fixup" happens and the property "ranges" is set
>> again, this time with only one entry (for the nor flash). All other
>> entries are lost.
>>
>
> I see. I didn't spot this problem in the original patch. Thanks for catching.
>
> It shouldn't be too hard to fix this problem by reading the complete ranges
> array back and patching only the first entry and re-writing ranges again.
>
>
I think my first patch that open-coded part of ft_board_setup() related
to EBC ranges
was correct and tested on real hardware. I admit that I didn't test the
second version
of the patch, as I didn't have the hardware at hand.
I'll submit a fix soon, but I still don't have hardware for testing, so
I'd appreciate your
help with that.
Felix.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
On Wednesday 15 July 2009 09:28:58 Eibach, Dirk wrote:
> > Please check the latest version of this code in
> > canyonlands.c. It's now using the 4xx common
> > __ft_board_setup() function. Please move to this version as well.
>
> This code in canyonlands.c does not work for me:
>
> void ft_board_setup(void *blob, bd_t *bd)
> {
> u32 val[4];
> int rc;
>
> __ft_board_setup(blob, bd);
>
> /* Fixup NOR mapping */
> val[0] = CONFIG_SYS_NOR_CS; /* chip select number */
> val[1] = 0; /* always 0 */
> val[2] = CONFIG_SYS_FLASH_BASE_PHYS_L; /* we fixed up
> this address */
> val[3] = gd->bd->bi_flashsize;
> rc = fdt_find_and_setprop(blob, "/plb/opb/ebc", "ranges",
> val, sizeof(val), 1);
>
> __ft_board_setup() sets the property "ranges", with an entry for every
> member of EBC_NUM_BANKS.
> Then the so called "fixup" happens and the property "ranges" is set
> again, this time with only one entry (for the nor flash). All other
> entries are lost.
I see. I didn't spot this problem in the original patch. Thanks for catching.
It shouldn't be too hard to fix this problem by reading the complete ranges
array back and patching only the first entry and re-writing ranges again.
Thanks.
Best regards,
Stefan
=
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
> > diff --git a/board/gdsys/compactcenter/compactcenter.c
> > b/board/gdsys/compactcenter/compactcenter.c new file mode
> 100644 index
> > 000..9f1e49d
> > --- /dev/null
> > +++ b/board/gdsys/compactcenter/compactcenter.c
>
>
>
> > +#if defined(CONFIG_OF_LIBFDT) &&
> defined(CONFIG_OF_BOARD_SETUP) void
> > +ft_board_setup(void *blob, bd_t *bd) {
>
> Please check the latest version of this code in
> canyonlands.c. It's now using the 4xx common
> __ft_board_setup() function. Please move to this version as well.
This code in canyonlands.c does not work for me:
void ft_board_setup(void *blob, bd_t *bd)
{
u32 val[4];
int rc;
__ft_board_setup(blob, bd);
/* Fixup NOR mapping */
val[0] = CONFIG_SYS_NOR_CS; /* chip select number */
val[1] = 0; /* always 0 */
val[2] = CONFIG_SYS_FLASH_BASE_PHYS_L; /* we fixed up
this address */
val[3] = gd->bd->bi_flashsize;
rc = fdt_find_and_setprop(blob, "/plb/opb/ebc", "ranges",
val, sizeof(val), 1);
__ft_board_setup() sets the property "ranges", with an entry for every
member of EBC_NUM_BANKS.
Then the so called "fixup" happens and the property "ranges" is set
again, this time with only one entry (for the nor flash). All other
entries are lost.
Cheers
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ppc4xx: Add GDsys CompactCenter board support.
Hi Dirk,
On Monday 13 July 2009 10:35:58 Dirk Eibach wrote:
> diff --git a/board/gdsys/compactcenter/compactcenter.c
> b/board/gdsys/compactcenter/compactcenter.c new file mode 100644
> index 000..9f1e49d
> --- /dev/null
> +++ b/board/gdsys/compactcenter/compactcenter.c
> +int get_cpu_num(void)
> +{
> + int cpu = NA_OR_UNKNOWN_CPU;
> +
> + return cpu;
> +}
Do you really need this function?
> +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
> +void ft_board_setup(void *blob, bd_t *bd)
> +{
Please check the latest version of this code in canyonlands.c. It's now using
the 4xx common __ft_board_setup() function. Please move to this version as
well.
> +++ b/include/configs/compactcenter.h
> @@ -0,0 +1,495 @@
> +/*
> + * (C) Copyright 2009
> + * Dirk Eibach, Guntermann & Drunck GmbH, eib...@gdsys.de
> + *
> + * Based on include/configs/canyonlands.h
> + * (C) Copyright 2008
> + * Stefan Roese, DENX Software Engineering, s...@denx.de.
> + *
> + * 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
> + */
> +
> +/*
> + * compactcenter.h - configuration for CompactCenter (460EX)
> + */
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +/*
> + * High Level Configuration Options
> + */
> +/*
> + * This config file is used for Canyonlands and DevCon-Center
Canyonlands?
> +/*
> + * PPC4xx GPIO Configuration
> + */
> +/* 460EX: Use USB configuration */
> +#define CONFIG_SYS_4xx_GPIO_TABLE { \
> +/* Out Alternate1 Alternate2 Alternate3 */ \
> +{ \
> +/* GPIO Core 0 */ \
> +/* GPIO0 GMC1TxD(0) USB2HostD(0) */ \
> +{GPIO0_BASE, GPIO_BI , GPIO_ALT1, GPIO_OUT_0}, \
I prefer to not split the too long lines in this CONFIG_SYS_4xx_GPIO_TABLE
macro. I know that the resulting lines are > 80 chars, but it really is *much*
better readable and maintainable this way.
Best regards,
Stefan
=
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
