Re: [U-Boot] [PATCH 03/11] video: Add System-Mode configuration hook into mxsfb

[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 <ma...@denx.de>
> Cc: Anatolij Gustschin <ag...@denx.de>
> Cc: Fabio Estevam <fabio.este...@freescale.com>
> Cc: Otavio Salvador <ota...@ossystems.com.br>
> Cc: Stefano Babic <sba...@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 ?

>  /*
>   * 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...

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