Hi Pardeep, On 09/07/2013 23:58, Pardeep Kumar Singla wrote: > Signed-off-by: Pardeep Kumar Singla <b45...@freescale.com> > --- > board/freescale/mx6qsabresd/mx6qsabresd.c | 92 > ++++++++++++++++++++++++++++- > include/configs/mx6qsabre_common.h | 3 +- > include/configs/mx6qsabresd.h | 13 ++++ > 3 files changed, 106 insertions(+), 2 deletions(-) >
Your patch shares a lot of code with mx6qsabrelite. Can we factorize the common code ? > diff --git a/board/freescale/mx6qsabresd/mx6qsabresd.c > b/board/freescale/mx6qsabresd/mx6qsabresd.c > index 2529826..301fd1b 100644 > --- a/board/freescale/mx6qsabresd/mx6qsabresd.c > +++ b/board/freescale/mx6qsabresd/mx6qsabresd.c > @@ -31,6 +31,11 @@ > #include <fsl_esdhc.h> > #include <miiphy.h> > #include <netdev.h> > +#include <asm/arch/crm_regs.h> > +#include <asm/arch/crm_regs.h> > +#include <ipu_pixfmt.h> > +#include <linux/fb.h> > +#include <asm/arch/mxc_hdmi.h> > > DECLARE_GLOBAL_DATA_PTR; > > @@ -133,6 +138,80 @@ static void setup_iomux_uart(void) > imx_iomux_v3_setup_multiple_pads(uart1_pads, ARRAY_SIZE(uart1_pads)); > } > > +#if defined(CONFIG_VIDEO_IPUV3) > +static void enable_hdmi(void) > +{ > + struct hdmi_regs *hdmi = (struct hdmi_regs *)HDMI_ARB_BASE_ADDR; > + u8 reg; > + reg = readb(&hdmi->phy_conf0); > + reg |= HDMI_PHY_CONF0_PDZ_MASK; > + writeb(reg, &hdmi->phy_conf0); > + udelay(3000); > + reg |= HDMI_PHY_CONF0_ENTMDS_MASK; > + writeb(reg, &hdmi->phy_conf0); > + udelay(3000); > + reg |= HDMI_PHY_CONF0_GEN2_TXPWRON_MASK; > + writeb(reg, &hdmi->phy_conf0); > + writeb(HDMI_MC_PHYRSTZ_ASSERT, &hdmi->mc_phyrstz); > +} > + For example, enable:hdmi is really identical to the same function of the sabrelite. > +static struct fb_videomode const hdmi = { > + .name = "HDMI", > + .refresh = 60, > + .xres = 1024, > + .yres = 768, > + .pixclock = 15385, > + .left_margin = 220, > + .right_margin = 40, > + .upper_margin = 21, > + .lower_margin = 7, > + .hsync_len = 60, > + .vsync_len = 10, > + .sync = FB_SYNC_EXT, > + .vmode = FB_VMODE_NONINTERLACED > +}; > + > +int board_video_skip(void) > +{ > + int ret; > + ret = ipuv3_fb_init(&hdmi, 0, IPU_PIX_FMT_RGB24); > + if (ret) > + printf("HDMI cannot be configured: %d\n", ret); > + enable_hdmi(); > + return ret; > +} > + > +static void setup_display(void) > +{ > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > + struct hdmi_regs *hdmi = (struct hdmi_regs *)HDMI_ARB_BASE_ADDR; > + int reg; > + > + /* Turn on IPU clock */ > + reg = readl(&mxc_ccm->CCGR3); > + reg |= MXC_CCM_CCGR3_IPU1_IPU_DI0_OFFSET; > + writel(reg, &mxc_ccm->CCGR3); > + /* Turn on HDMI PHY clock */ > + reg = readl(&mxc_ccm->CCGR2); > + reg |= MXC_CCM_CCGR2_HDMI_TX_IAHBCLK_MASK| > + MXC_CCM_CCGR2_HDMI_TX_ISFRCLK_MASK; > + writel(reg, &mxc_ccm->CCGR2); > + /* clear HDMI PHY reset */ > + writeb(HDMI_MC_PHYRSTZ_DEASSERT, &hdmi->mc_phyrstz); > + reg = readl(&mxc_ccm->chsccdr); > + reg &= ~(MXC_CCM_CHSCCDR_IPU1_DI0_PRE_CLK_SEL_MASK| > + MXC_CCM_CHSCCDR_IPU1_DI0_PODF_MASK| > + MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_MASK); > + reg |= (CHSCCDR_CLK_SEL_LDB_DI0 > + << MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_OFFSET)| > + (CHSCCDR_PODF_DIVIDE_BY_3 > + << MXC_CCM_CHSCCDR_IPU1_DI0_PODF_OFFSET) > + | (CHSCCDR_IPU_PRE_CLK_540M_PFD > + << MXC_CCM_CHSCCDR_IPU1_DI0_PRE_CLK_SEL_OFFSET); > + writel(reg, &mxc_ccm->chsccdr); > +} > +#endif /* CONFIG_VIDEO_IPUV3 */ setup_display has also some common parts with the same function for sabrelite. Any way to factorize code ? Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot