From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 31 Jul 2013 15:22:51 +0200
Subject: [U-Boot] [PATCH 03/11] video: Add System-Mode configuration
	hook into mxsfb
In-Reply-To: <51F90EE2.6010604@denx.de>
References: <1375220281-11132-1-git-send-email-marex@denx.de>
	<1375220281-11132-4-git-send-email-marex@denx.de>
	<51F90EE2.6010604@denx.de>
Message-ID: <201307311522.51669.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Stefano Babic,

> Hi Marek,
> 
> On 30/07/2013 23:37, Marek Vasut wrote:
> > Add hook that allow configuring SmartLCD attached the MXS LCDIF
> > controller operating in System-Mode. This hook can be overriden
> > by a platform-specific SmartLCD programming routine, which writes
> > the SmartLCD specific values into it's registers.
> > 
> > Also, this patch makes sure the SYNC signals are off for the
> > SmartLCD case.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Anatolij Gustschin <agust@denx.de>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Otavio Salvador <otavio@ossystems.com.br>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> > 
> >  drivers/video/mxsfb.c |   16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> > index dbc63a6..78709dd 100644
> > --- a/drivers/video/mxsfb.c
> > +++ b/drivers/video/mxsfb.c
> > @@ -34,6 +34,17 @@
> > 
> >  static GraphicDevice panel;
> > 
> > +/**
> > + * mxsfb_system_setup() - Fine-tune LCDIF configuration
> > + *
> > + * This function is used to adjust the LCDIF configuration. This is
> > usually + * needed when driving the controller in System-Mode to operate
> > an 8080 or + * 6800 connected SmartLCD.
> > + */
> > +__weak void mxsfb_system_setup(void)
> > +{
> > +}
> > +
> 
> We have no easy way to know if a function is declared weak, but
> generally a lot of weak functions are board specific.
> 
> Try to use the same naming schema, and rename this one as
> board_mxfb_setup() (or something like that).
> 
> Or better: why cannot we use board_video_init(), as this name is already
> used by other SOCs ?

This is system-mode specific and needs to be called at this particular place.

> >  /*
> >  
> >   * DENX M28EVK:
> >   * setenv videomode
> > 
> > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel,
> > 
> >  	writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET,
> >  	
> >  		&regs->hw_lcdif_ctrl1);
> > 
> > +
> > +	mxsfb_system_setup();
> > +
> > 
> >  	writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) |
> >  	mode->xres,
> >  	
> >  		&regs->hw_lcdif_transfer_count);
> > 
> > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel,
> > 
> >  	/* Flush FIFO first */
> >  	writel(LCDIF_CTRL1_FIFO_CLEAR, &regs->hw_lcdif_ctrl1_set);
> > 
> > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM
> > 
> >  	/* Sync signals ON */
> >  	setbits_le32(&regs->hw_lcdif_vdctrl4, LCDIF_VDCTRL4_SYNC_SIGNALS_ON);
> > 
> > +#endif
> 
> Question: is there anything wrong to move it into mxsfb_system_setup() ?
> I would prefer to avoid adding a new not documented CONFIG_ only to set
> a bit...

Yes, the order in which the LCD is configured must be preserved as-is.

Best regards,
Marek Vasut