[U-Boot] [PATCH 3/3] Kirkwood: Add support for Ka-Ro TK71

[Marek Vasut]

Dear Prafulla Wadaskar,

> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: 26 June 2012 17:45
> > To: u-boot at 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 <marex at denx.de>
> > Cc: Prafulla Wadaskar <prafulla at marvell.com>
> > Cc: Wolfgang Denk <wd at 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 <marex at 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