Guennadi Liakhovetski wrote:
> Add a driver for the Synchronous Display Controller and the Display
> Interface on i.MX31, using IPU for DMA channel setup. So far only
> displaying of bitmaps is supported, no text output.
>
> Signed-off-by: Guennadi Liakhovetski <l...@denx.de>
mostly looks good, please see some comments below.
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> new file mode 100644
> index 0000000..d9b673e
> --- /dev/null
> +++ b/drivers/video/mx3fb.c
<snip>
> +/**
> + * sdc_init_panel() - initialize a synchronous LCD panel.
> + * @mx3fb: mx3fb context.
> + * @panel: panel type.
> + * @pixel_clk: desired pixel clock frequency in Hz.
> + * @width: width of panel in pixels.
> + * @height: height of panel in pixels.
> + * @pixel_fmt: pixel format of buffer as FOURCC ASCII code.
> + * @h_start_width: number of pixel clocks between the HSYNC signal pulse
> + * and the start of valid data.
> + * @h_sync_width: width of the HSYNC signal in units of pixel clocks.
> + * @h_end_width: number of pixel clocks between the end of valid data
> + * and the HSYNC signal for next line.
> + * @v_start_width: number of lines between the VSYNC signal pulse and the
> + * start of valid data.
> + * @v_sync_width: width of the VSYNC signal in units of lines
> + * @v_end_width: number of lines between the end of valid data and the
> + * VSYNC signal for next frame.
> + * @sig: bitfield of signal polarities for LCD interface.
> + * @return: 0 on success or negative error code on failure.
> + */
> +static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt)
function parameters description is from the Linux driver,
however most of them are not present here in U-Boot driver.
Is there any reason you leave the original description here?
Also please replace starting '/**' with '/*'.
<snip>
> +static void ipu_ch_param_set_size(union chan_param_mem *params,
> + uint32_t pixel_fmt, uint16_t width,
> + uint16_t height, uint16_t stride)
> +{
> + params->pp.fw = width - 1;
> + params->pp.fh_l = height - 1;
> + params->pp.fh_h = (height - 1) >> 8;
> + params->pp.sl = stride - 1;
> +
> + /* See above, for further formats see tge Linux driver */
s/tge/the/
?
<snip>
> +/**
> + * ipu_enable_channel() - enable an IPU channel.
> + * @channel: channel ID.
> + * @return: 0 on success or negative error code on failure.
> + */
> +static int ipu_enable_channel(enum ipu_channel channel)
just use '/*' here, too.
<snip>
> +/**
> + * mx3fb_set_par() - set framebuffer parameters and change the operating
> mode.
> + * @fbi: framebuffer information pointer.
> + * @return: 0 on success or negative error code on failure.
> + */
> +static int mx3fb_set_par(void *fbmem)
use '/*' here, too. Thanks!
Also @fbi description doesn't apply here, 'fbmem' is frame buffer
memory pointer.
Best regards,
Anatolij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
