Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
Dear Mike Rapoport,
In message <4cff26b0.1080...@compulab.co.il> you wrote:
>
> This patch adds support for CM-T35 board
>
> Signed-off-by: Mike Rapoport
> ---
...
> diff --git a/MAKEALL b/MAKEALL
> index c54c6e8..e0fe12f 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -412,6 +412,7 @@ LIST_ARM11=" \
> LIST_ARMV7=" \
> am3517_evm \
> ca9x4_ct_vxp\
> + cm_t35 \
> devkit8000 \
> igep0020\
> igep0030\
Boards don't get added to MAKEALL any more. Please verify that your
board automatically gets picked up from boards.cfg
> +/*
> + * Routine: misc_init_r
> + * Description: Init ethernet (done here so udelay works)
> + */
> +int misc_init_r(void)
> +{
> +#ifdef CONFIG_DRIVER_OMAP34XX_I2C
> + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +#endif
> +
> + dieid_num_r();
> +
> + return 0;
> +}
Comment and code don't appear to match.
> +/*
> + * Routine: set_muxconf_regs
> + * Description: Setting up the configuration Mux registers specific to the
> + * hardware. Many pins need to be moved from protect to primary
> + * mode.
> + */
> +void set_muxconf_regs(void)
> +{
> + MUX_CM_T35();
> +}
Why don't you write this code here directly? Adding another macro
level here just obfuscates the code. Or what is your rationale for
moving this into a header file ?
> +#ifdef CONFIG_GENERIC_MMC
> +int board_mmc_init(bd_t *bis)
> +{
> + omap_mmc_init(0);
> + return 0;
Error handling?
> +static int handle_mac_address(void)
> +{
> + unsigned char enetaddr[6];
> + int rc;
> +
> + memset(enetaddr, 0, 6);
> + rc = eth_getenv_enetaddr("ethaddr", enetaddr);
What exactly is the memset() good for?
> diff --git a/board/cm_t35/config.mk b/board/cm_t35/config.mk
> new file mode 100644
> index 000..e81e283
> --- /dev/null
> +++ b/board/cm_t35/config.mk
...
> +CONFIG_SYS_TEXT_BASE = 0x80008000
Please move to board config file and get rid of config.mk
> --- /dev/null
> +++ b/include/configs/cm_t35.h
...
> +/*
> + * select serial console configuration
> + */
> +#define CONFIG_CONS_INDEX3
> +#define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3
> +#define CONFIG_SERIAL3 3 /* UART3 on Beagle Rev
> 2 */
Beagle Rev 2?
> +#undef CONFIG_CMD_IMI/* iminfo */
What is the exact reson for disabling iminfo ?
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
You see but you do not observe.
Sir Arthur Conan Doyle, in "The Memoirs of Sherlock Holmes"
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
Dear Wolfgang, Wolfgang Denk wrote: > Dear Mike Rapoport, > > In message <1257955131-16729-1-git-send-email-m...@compulab.co.il> you wrote: >> Add CM-T35 board support >> >> -- >> v2 changes: >> - rename board config file from omap3_cm-t35.h to cm-t35.h >> - remove SZ_xx references >> - add MAKEALL/MAINTEINERS entries >> -- >> >> Signed-off-by: Mike Rapoport > > The Signed-off-by: line belongs _above_ the "--" line. > >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -677,6 +677,10 @@ Stelian Pop >> at91sam9263ek ARM926EJS (AT91SAM9263 SoC) >> at91sam9rlekARM926EJS (AT91SAM9RL SoC) >> >> +Mike Rapoport >> + >> +omap3_cm-t35ARM CORTEX-A8 (OMAP3xx SoC) > > Please fix the board name. > >> diff --git a/board/cm-t35/cm-t35.c b/board/cm-t35/cm-t35.c >> new file mode 100644 >> index 000..b3eb087 >> --- /dev/null >> +++ b/board/cm-t35/cm-t35.c >> +udelay(1); >> +twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, >> + TWL4030_BASEADD_GPIO+0x0C); >> + >> +} > > Delete this empty line, please (please check globally). > > >> +++ b/board/cm-t35/cm-t35.h > ... >> +#define MUX_CM_T35() \ >> + /*SDRC*/\ >> + MUX_VAL(CP(SDRC_D0), (IEN | PTD | DIS | M0)) /*SDRC_D0*/\ > ... > Indentation by TAB, please. > >> +++ b/board/cm-t35/config.mk >> @@ -0,0 +1,30 @@ > ... >> +# For use with external or internal boots. >> +TEXT_BASE = 0x80e8 >> \ No newline at end of file > ^^ > > Please fix. > >> diff --git a/include/configs/cm-t35.h b/include/configs/cm-t35.h >> new file mode 100755 >> index 000..b881112 >> --- /dev/null >> +++ b/include/configs/cm-t35.h > ... >> +#ifndef __ASSEMBLY__ >> +extern struct gpmc *gpmc_cfg; >> +extern unsigned int boot_flash_base; >> +extern volatile unsigned int boot_flash_env_addr; >> +extern unsigned int boot_flash_off; >> +extern unsigned int boot_flash_sec; >> +extern unsigned int boot_flash_type; >> +#endif > > These should not be needed in your board config file. Please move to a > more appropriate header. This is what all other omap3 board do... Moving these requires some rework of common omap3 code and updates to all omap3 boards. > > Best regards, > > Wolfgang Denk > -- Sincerely yours, Mike. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
On Wed, Nov 11, 2009 at 10:03 PM, Ben Warren wrote:
> Mike Rapoport wrote:
>> Add CM-T35 board support
>>
>> --
>> v2 changes:
>> - rename board config file from omap3_cm-t35.h to cm-t35.h
>> - remove SZ_xx references
>> - add MAKEALL/MAINTEINERS entries
>> --
>>
>>
>
>> --- /dev/null
>> +++ b/board/cm-t35/cm-t35.c
>> @@ -0,0 +1,196 @@
>> +/*
>> + * (C) Copyright 2009
>> + * CompuLab, Ltd.
>> + *
>> + * Authors :
>> + * Igor Vaisbein
>> + * Mike Rapoport
>> + *
>> + * Derived from omap3evm and Beagle Board by
>> + * Manikandan Pillai
>> + * Richard Woodruff
>> + * Syed Mohammed Khasim
>> + *
>> + * 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., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "cm-t35.h"
>> +
>> +#define CM_T35_SMC911X_BASE 0x2C00
>> +#define SB_T35_SMC911X_BASE (CM_T35_SMC911X_BASE + (16 << 20))
>>
> Please follow convention and put this stuff in include/configs/board.h
>> +
>> +static u32 gpmc_net_config[GPMC_MAX_REG] = {
>> + NET_GPMC_CONFIG1,
>> + NET_GPMC_CONFIG2,
>> + NET_GPMC_CONFIG3,
>> + NET_GPMC_CONFIG4,
>> + NET_GPMC_CONFIG5,
>> + NET_GPMC_CONFIG6,
>> + 0
>> +};
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +/*
>> + * Routine: setup_net_chip
>> + * Description: Setting up the configuration GPMC registers specific to the
>> + * Ethernet hardware.
>> + */
>> +static void setup_net_chip(void)
>> +{
>> + struct ctrl *ctrl_base = (struct ctrl *)OMAP34XX_CTRL_BASE;
>> +
>> + enable_gpmc_cs_config(gpmc_net_config, &gpmc_cfg->cs[5],
>> + CM_T35_SMC911X_BASE, GPMC_SIZE_16M);
>> + enable_gpmc_cs_config(gpmc_net_config, &gpmc_cfg->cs[4],
>> + SB_T35_SMC911X_BASE, GPMC_SIZE_16M);
>> +
>> + /* Enable off mode for NWE in PADCONF_GPMC_NWE register */
>> + writew(readw(&ctrl_base->gpmc_nwe) | 0x0E00, &ctrl_base->gpmc_nwe);
>> +
>> + /* Enable off mode for NOE in PADCONF_GPMC_NADV_ALE register */
>> + writew(readw(&ctrl_base->gpmc_noe) | 0x0E00, &ctrl_base->gpmc_noe);
>> +
>> + /* Enable off mode for ALE in PADCONF_GPMC_NADV_ALE register */
>> + writew(readw(&ctrl_base->gpmc_nadv_ale) | 0x0E00,
>> + &ctrl_base->gpmc_nadv_ale);
>> +
>> + /* Reset the ethernet controller via TPS65930 GPIO */
>> + /* Set GPIO1 of TPS65930 as output */
>> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
>> + TWL4030_BASEADD_GPIO+0x03);
>> + /* Send a pulse on the GPIO pin */
>> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
>> + TWL4030_BASEADD_GPIO+0x0C);
>> + udelay(1);
>> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
>> + TWL4030_BASEADD_GPIO+0x09);
>> + udelay(1);
>> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
>> + TWL4030_BASEADD_GPIO+0x0C);
>> +
>> +}
>> +
>> +/*
>> + * Routine: board_init
>> + * Description: Early hardware init.
>> + */
>> +int board_init(void)
>> +{
>> + gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
>> +
>> + /* board id for Linux */
>> + gd->bd->bi_arch_number = MACH_TYPE_CM_T35;
>> + /* boot param addr */
>> + gd->bd->bi_boot_params = (OMAP34XX_SDRC_CS0 + 0x100);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Routine: misc_init_r
>> + * Description: Init ethernet (done here so udelay works)
>> + */
>> +int misc_init_r(void)
>> +{
>> +
>> +#ifdef CONFIG_DRIVER_OMAP34XX_I2C
>> + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +#endif
>> +
>> +#if defined(CONFIG_CMD_NET)
>> + setup_net_chip();
>> +#endif
>> +
>> + dieid_num_r();
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Routine: set_muxconf_regs
>> + * Description: Setting up the configuration Mux registers specific to the
>> + * hardware. Many pins need to be moved from protect to primary
>> + * mode.
>> + */
>> +void set_muxconf_regs(void)
>> +{
>> + MUX_CM_T35();
>> +}
>> +
>> +/*
>> + * Routine: handle_mac_address
>> + * Description: prepare MAC address f
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
Dear Mike Rapoport, In message <1257955131-16729-1-git-send-email-m...@compulab.co.il> you wrote: > Add CM-T35 board support > > -- > v2 changes: > - rename board config file from omap3_cm-t35.h to cm-t35.h > - remove SZ_xx references > - add MAKEALL/MAINTEINERS entries > -- > > Signed-off-by: Mike Rapoport The Signed-off-by: line belongs _above_ the "--" line. > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -677,6 +677,10 @@ Stelian Pop > at91sam9263ek ARM926EJS (AT91SAM9263 SoC) > at91sam9rlekARM926EJS (AT91SAM9RL SoC) > > +Mike Rapoport > + > + omap3_cm-t35ARM CORTEX-A8 (OMAP3xx SoC) Please fix the board name. > diff --git a/board/cm-t35/cm-t35.c b/board/cm-t35/cm-t35.c > new file mode 100644 > index 000..b3eb087 > --- /dev/null > +++ b/board/cm-t35/cm-t35.c > + udelay(1); > + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, > + TWL4030_BASEADD_GPIO+0x0C); > + > +} Delete this empty line, please (please check globally). > +++ b/board/cm-t35/cm-t35.h ... > +#define MUX_CM_T35() \ > + /*SDRC*/\ > + MUX_VAL(CP(SDRC_D0),(IEN | PTD | DIS | M0)) /*SDRC_D0*/\ ... Indentation by TAB, please. > +++ b/board/cm-t35/config.mk > @@ -0,0 +1,30 @@ ... > +# For use with external or internal boots. > +TEXT_BASE = 0x80e8 > \ No newline at end of file ^^ Please fix. > diff --git a/include/configs/cm-t35.h b/include/configs/cm-t35.h > new file mode 100755 > index 000..b881112 > --- /dev/null > +++ b/include/configs/cm-t35.h ... > +#ifndef __ASSEMBLY__ > +extern struct gpmc *gpmc_cfg; > +extern unsigned int boot_flash_base; > +extern volatile unsigned int boot_flash_env_addr; > +extern unsigned int boot_flash_off; > +extern unsigned int boot_flash_sec; > +extern unsigned int boot_flash_type; > +#endif These should not be needed in your board config file. Please move to a more appropriate header. 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 Conquest is easy. Control is not. -- Kirk, "Mirror, Mirror", stardate unknown ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
On Wed, Nov 11, 2009 at 8:37 PM, Dirk Behme wrote: > Mike Rapoport wrote: >> Add CM-T35 board support >> >> -- >> v2 changes: >> - rename board config file from omap3_cm-t35.h to cm-t35.h >> - remove SZ_xx references >> - add MAKEALL/MAINTEINERS entries > > Sorry, you are too fast for me ;) > > I wanted to mention to please update doc/README.omap3, too. WIll do. > Cheers > > Dirk > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > -- Sincerely Yours, Mike. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
Mike Rapoport wrote:
> Add CM-T35 board support
>
> --
> v2 changes:
> - rename board config file from omap3_cm-t35.h to cm-t35.h
> - remove SZ_xx references
> - add MAKEALL/MAINTEINERS entries
> --
>
>
> --- /dev/null
> +++ b/board/cm-t35/cm-t35.c
> @@ -0,0 +1,196 @@
> +/*
> + * (C) Copyright 2009
> + * CompuLab, Ltd.
> + *
> + * Authors :
> + * Igor Vaisbein
> + * Mike Rapoport
> + *
> + * Derived from omap3evm and Beagle Board by
> + * Manikandan Pillai
> + * Richard Woodruff
> + * Syed Mohammed Khasim
> + *
> + * 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., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "cm-t35.h"
> +
> +#define CM_T35_SMC911X_BASE 0x2C00
> +#define SB_T35_SMC911X_BASE (CM_T35_SMC911X_BASE + (16 << 20))
>
Please follow convention and put this stuff in include/configs/board.h
> +
> +static u32 gpmc_net_config[GPMC_MAX_REG] = {
> + NET_GPMC_CONFIG1,
> + NET_GPMC_CONFIG2,
> + NET_GPMC_CONFIG3,
> + NET_GPMC_CONFIG4,
> + NET_GPMC_CONFIG5,
> + NET_GPMC_CONFIG6,
> + 0
> +};
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * Routine: setup_net_chip
> + * Description: Setting up the configuration GPMC registers specific to the
> + * Ethernet hardware.
> + */
> +static void setup_net_chip(void)
> +{
> + struct ctrl *ctrl_base = (struct ctrl *)OMAP34XX_CTRL_BASE;
> +
> + enable_gpmc_cs_config(gpmc_net_config, &gpmc_cfg->cs[5],
> + CM_T35_SMC911X_BASE, GPMC_SIZE_16M);
> + enable_gpmc_cs_config(gpmc_net_config, &gpmc_cfg->cs[4],
> + SB_T35_SMC911X_BASE, GPMC_SIZE_16M);
> +
> + /* Enable off mode for NWE in PADCONF_GPMC_NWE register */
> + writew(readw(&ctrl_base->gpmc_nwe) | 0x0E00, &ctrl_base->gpmc_nwe);
> +
> + /* Enable off mode for NOE in PADCONF_GPMC_NADV_ALE register */
> + writew(readw(&ctrl_base->gpmc_noe) | 0x0E00, &ctrl_base->gpmc_noe);
> +
> + /* Enable off mode for ALE in PADCONF_GPMC_NADV_ALE register */
> + writew(readw(&ctrl_base->gpmc_nadv_ale) | 0x0E00,
> + &ctrl_base->gpmc_nadv_ale);
> +
> + /* Reset the ethernet controller via TPS65930 GPIO */
> + /* Set GPIO1 of TPS65930 as output */
> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
> + TWL4030_BASEADD_GPIO+0x03);
> + /* Send a pulse on the GPIO pin */
> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
> + TWL4030_BASEADD_GPIO+0x0C);
> + udelay(1);
> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
> + TWL4030_BASEADD_GPIO+0x09);
> + udelay(1);
> + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02,
> + TWL4030_BASEADD_GPIO+0x0C);
> +
> +}
> +
> +/*
> + * Routine: board_init
> + * Description: Early hardware init.
> + */
> +int board_init(void)
> +{
> + gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
> +
> + /* board id for Linux */
> + gd->bd->bi_arch_number = MACH_TYPE_CM_T35;
> + /* boot param addr */
> + gd->bd->bi_boot_params = (OMAP34XX_SDRC_CS0 + 0x100);
> +
> + return 0;
> +}
> +
> +/*
> + * Routine: misc_init_r
> + * Description: Init ethernet (done here so udelay works)
> + */
> +int misc_init_r(void)
> +{
> +
> +#ifdef CONFIG_DRIVER_OMAP34XX_I2C
> + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +#endif
> +
> +#if defined(CONFIG_CMD_NET)
> + setup_net_chip();
> +#endif
> +
> + dieid_num_r();
> +
> + return 0;
> +}
> +
> +/*
> + * Routine: set_muxconf_regs
> + * Description: Setting up the configuration Mux registers specific to the
> + * hardware. Many pins need to be moved from protect to primary
> + * mode.
> + */
> +void set_muxconf_regs(void)
> +{
> + MUX_CM_T35();
> +}
> +
> +/*
> + * Routine: handle_mac_address
> + * Description: prepare MAC address for on-board Ethernet.
> + */
> +static int handle_mac_address(void)
> +{
> + unsigned char enetaddr[6];
> + int rc;
> +
> + rc = eth_getenv_enetaddr("ethaddr", enetaddr);
> + if (rc)
> + ret
Re: [U-Boot] [PATCH v2] OMAP3: add CM-T35 board
Mike Rapoport wrote: > Add CM-T35 board support > > -- > v2 changes: > - rename board config file from omap3_cm-t35.h to cm-t35.h > - remove SZ_xx references > - add MAKEALL/MAINTEINERS entries Sorry, you are too fast for me ;) I wanted to mention to please update doc/README.omap3, too. Cheers Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
