On Friday 20 August 2010 11:51:18 am Stefano Babic wrote: > Hi David, > > in addition to Wolfgang's comments: > > +static u32 system_rev; > > +struct io_board_ctrl *mx51_io_board; > > Structure is not used, and probably does not match your board. You > should drop it.
Ok, I was already wondering what this did here.... I'll just remove it. > > +#define POUT_HS (PAD_CTL_DRV_HIGH | PAD_CTL_SRE_FAST) > > +#define POUT_MS (PAD_CTL_DRV_MAX | PAD_CTL_SRE_FAST) > > +#define POUT_LS (PAD_CTL_DRV_MEDIUM) > > +#define PIN_HYS (PAD_CTL_HYS_ENABLE) > > +#define PIN_HYS_PULL (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | > > PAD_CTL_PUE_PULL) +#define PIN_HYS_KEEP (PAD_CTL_HYS_ENABLE | > > PAD_CTL_PKE_ENABLE) > > +#define PIO_OD (PIN_HYS_PULL | PAD_CTL_22K_PU | > > PAD_CTL_ODE_OPENDRAIN_ENABLE | PAD_CTL_DRV_MEDIUM) > > Consider to put these defines in include/asm/arch-mx51/iomux.h. They > could be usefult for other boards, too. Hmmm. In that case they should have other names, and one should probably make the set complete with all combinations, and I doubt it'll make sense anymore then.... This subset of possibilities is too specific to our board I fear. I had already been complaining about some seemingly arbitrary combinations of IO-pad settings being used in SoC-driver code in the linux kernel. IMHO, one should never choose a specific combination of settings without excactly knowing it is electrically correct for that specific pin on that specific board... not only because it works, but also from EMC considerations. I also think, that in theory this would make the kernel board-specific in that case... not something one would want I believe. For that reason, I keep thinking that correct and thorough IO-pad setup specific for each board should be done in the boot-loader.... never in the kernel. OTOH, maybe one could come up with some commonly-used combinations... but which ones? > > + /* Raise the core frequency to 800MHz */ > > + /* printf("Core at 400 MHz!\n"); */ > > + /* writel(0x1, &mxc_ccm->cacrr); */ > > + writel(0x0, &mxc_ccm->cacrr); > > Comment is misleading. Remove what is not needed. Sorry, I was too lazy to make a configuration option out of this... will fix it. > > + /* Setup other io's */ > > + for(t=0; other_io_conf[t].pin>=0; t++) { > > Add spaces when needed. Ok. > > +#ifdef BOARD_LATE_INIT > > Probably it is better (and in mx51evk, too) to remove the #ifdef, > because we need to initialize the pmic, else some functionalities cannot > work. It is mandatory that power_init is called. I agree. I had always been "about to remove it" ;-) > > diff --git a/board/Protonic/prtlvt2/prtlvt2.h > > b/board/Protonic/prtlvt2/prtlvt2.h +#ifndef > > __BOARD_FREESCALE_MX51_EVK_H__ > > +#define __BOARD_FREESCALE_MX51_EVK_H__ > > + > > +#ifndef __ASSEMBLY__ > > +struct io_board_ctrl { > > + u16 led_ctrl; /* 0x00 */ > > + u16 resv1[0x03]; > > + u16 sb_stat; /* 0x08 */ > > + u16 resv2[0x03]; > > + u16 int_stat; /* 0x10 */ > > + u16 resv3[0x07]; > > + u16 int_rest; /* 0x20 */ > > + u16 resv4[0x0B]; > > + u16 int_mask; /* 0x38 */ > > + u16 resv5[0x03]; > > + u16 id1; /* 0x40 */ > > + u16 resv6[0x03]; > > + u16 id2; /* 0x48 */ > > + u16 resv7[0x03]; > > + u16 version; /* 0x50 */ > > + u16 resv8[0x03]; > > + u16 id3; /* 0x58 */ > > + u16 resv9[0x03]; > > + u16 sw_reset; /* 0x60 */ > > +}; > > +#endif > > Is this structure really used ? I have not seen in code. Or does it come > only from mx51evk ? You can remove the whole file, if it is not needed. No idea what it does. It's copied from MX51EVK. I will try removing it. Best regards, -- David Jander Protonic Holland. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot