Dear Prafulla Wadaskar,

> > -----Original Message-----
> > From: Marek Vasut [mailto:ma...@denx.de]
> > Sent: 26 June 2012 17:45
> > To: u-boot@lists.denx.de
> > Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
> > Subject: [PATCH 3/3] Kirkwood: Add support for Ka-Ro TK71
> > 
> > Signed-off-by: Marek Vasut <ma...@denx.de>
> > Cc: Prafulla Wadaskar <prafu...@marvell.com>
> > Cc: Wolfgang Denk <w...@denx.de>
> > ---
> > 
> >  board/karo/tk71/Makefile     |   45 +++++++++++
> >  board/karo/tk71/kwbimage.cfg |  174
> > 
> > +++++++++++++++++++++++++++++++++++++++++
> > 
> >  board/karo/tk71/tk71.c       |  178
> > 
> > ++++++++++++++++++++++++++++++++++++++++++
> > 
> >  boards.cfg                   |    1 +
> >  include/configs/tk71.h       |  128 ++++++++++++++++++++++++++++++
> >  5 files changed, 526 insertions(+)
> >  create mode 100644 board/karo/tk71/Makefile
> >  create mode 100644 board/karo/tk71/kwbimage.cfg
> >  create mode 100644 board/karo/tk71/tk71.c
> >  create mode 100644 include/configs/tk71.h
> 
> ...snip...
> 
> > diff --git a/board/karo/tk71/tk71.c b/board/karo/tk71/tk71.c
> > new file mode 100644
> > index 0000000..b2c1a62
> > --- /dev/null
> > +++ b/board/karo/tk71/tk71.c
> > @@ -0,0 +1,178 @@
> > +/*
> > + * Copyright (C) 2012 Marek Vasut <ma...@denx.de>
> > + * on behalf of DENX Software Engineering GmbH
> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * 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., 51 Franklin Street, Fifth Floor, Boston,
> > + * MA 02110-1301 USA
> > + */
> > +
> > +#include <common.h>
> > +#include <miiphy.h>
> > +#include <asm/arch/cpu.h>
> > +#include <asm/arch/kirkwood.h>
> > +#include <asm/arch/mpp.h>
> > +#include <asm/io.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define TK71_OE_LOW                        (~0)
> > +#define TK71_OE_HIGH                       (~0)
> > +#define TK71_OE_VAL_LOW                    (0)
> > +#define TK71_OE_VAL_HIGH           (0)
> > +
> > +int board_early_init_f(void)
> > +{
> > +   /*
> > +    * default gpio configuration
> > +    * There are maximum 64 gpios controlled through 2 sets of
> > registers
> > +    * the  below configuration configures mainly initial LED status
> > +    */
> > +   kw_config_gpio(TK71_OE_VAL_LOW,
> > +                   TK71_OE_VAL_HIGH,
> > +                   TK71_OE_LOW, TK71_OE_HIGH);
> > +
> > +   /* Multi-Purpose Pins Functionality configuration */
> > +   u32 kwmpp_config[] = {
> > +           MPP0_NF_IO2,
> > +           MPP1_NF_IO3,
> > +           MPP2_NF_IO4,
> > +           MPP3_NF_IO5,
> > +           MPP4_NF_IO6,
> > +           MPP5_NF_IO7,
> > +           MPP6_SYSRST_OUTn,
> > +           MPP7_GPO,
> > +           MPP8_TW_SDA,
> > +           MPP9_TW_SCK,
> > +           MPP10_UART0_TXD,
> > +           MPP11_UART0_RXD,
> > +           MPP12_SD_CLK,
> > +           MPP13_SD_CMD,
> > +           MPP14_SD_D0,
> > +           MPP15_SD_D1,
> > +           MPP16_SD_D2,
> > +           MPP17_SD_D3,
> > +           MPP18_NF_IO0,
> > +           MPP19_NF_IO1,
> > +           MPP20_GE1_0,
> > +           MPP21_GE1_1,
> > +           MPP22_GE1_2,
> > +           MPP23_GE1_3,
> > +           MPP24_GE1_4,
> > +           MPP25_GE1_5,
> > +           MPP26_GE1_6,
> > +           MPP27_GE1_7,
> > +           MPP28_GPIO,
> > +           MPP29_GPIO,
> > +           MPP30_GE1_10,
> > +           MPP31_GE1_11,
> > +           MPP32_GE1_12,
> > +           MPP33_GE1_13,
> > +           MPP34_GPIO,
> > +           MPP35_GPIO,
> > +           MPP36_GPIO,
> > +           MPP37_GPIO,
> > +           MPP38_GPIO,
> > +           MPP39_GPIO,
> > +           MPP40_GPIO,
> > +           MPP41_GPIO,
> > +           MPP42_GPIO,
> > +           MPP43_GPIO,
> > +           MPP44_GPIO,
> > +           MPP45_GPIO,
> > +           MPP46_GPIO,
> > +           MPP47_GPIO,
> > +           MPP48_GPIO,
> > +           MPP49_GPIO,
> > +           0
> > +   };
> > +   kirkwood_mpp_conf(kwmpp_config);
> > +
> > +   return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +   /*
> > +    * arch number of board
> > +    */
> > +   gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
> > +
> > +   /* adress of boot parameters */
> > +   gd->bd->bi_boot_params = kw_sdram_bar(0) + 0x100;
> > +
> > +   return 0;
> > +}
> > +
> > +void kw_adjust_dram(void)
> > +{
> > +   unsigned long size = get_ram_size(PHYS_SDRAM_1,
> > PHYS_SDRAM_1_SIZE);
> > +
> > +   /* 256MB module, adjust BAR register */
> > +   if (size == 256 * 1024 * 1024)
> > +           writel(0x0ffffff1, KW_REG_CPUCS_WIN_SZ(0));
> 
> No hard coding please, please use register structures (preferred)

Do we have any such structures defined ?

> > +
> > +   gd->bd->bi_dram[0].size = size;
> > +   gd->ram_size = size;
> > +}
> 
> You have created kwbimage.cfg in this, it makes more sense to set 256MB
> size by default in kwbimage.cfg. Or preferred use any existing
> kwbimage.cfg (preferred)

I don't think I follow. I always believe these are for setting up the platform, 
so at least one per platform is necessary.

> The main idea of having such weak functions is to avoid code duplication
> (kwbimage.cfg) for small manageable changes.

But then, isn't this a bit out of scope of this patchset? Unifying kwbimages I 
mean ...

> Regards..
> Prafulla . . .

Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to