On Mon, 2015-08-03 at 23:54 +0200, Hans de Goede wrote:
> @@ -775,13 +781,18 @@ static void sunxi_lcdc_tcon1_mode_set(const
> struct ctfb_res_modes *mode,
>
> clk_delay = sunxi_lcdc_get_clk_delay(mode, 1);
> writel(SUNXI_LCDC_TCON1_CTRL_ENABLE |
> + SUNXI_LCDC_TCON1_CTRL_INTERLACE(
> + mode->vmode == FB_VMODE_INTERLACED) |
I think this would be clearer if SUNXI_LCDC_TCON1_CTRL_INTERLACE was
actually the enable bit (perhaps with _ENABLE on the name), rather than
a macro which takes a boolean and returns 0 or the single bit, so you
could just write
mode->vmode == FB_VMODE_INTERLACED ?
SUNXI_LCDC_TCON1_CTRL_INTERLACE : 0
(in whichever wrapping style you prefer).
I think the macro is the bit style is more common in this code for
boolean options too, we mainly use the macro-with-argument style for
fields with more than 1 bit to them.
But ultimately the code is correct as you have it so either way as you
prefer:
Acked-by: Ian Campbell <[email protected]>
Although if you want to keep it the way it is then perhaps the macro sh
ould have !!n instead of just n, to prevent surprises if someone uses a
bitop rather than a full boolean op as an argument?
@@ -1240,6 +1245,9 @@ void *video_hw_init(void)
> return NULL;
> }
>
> +> > printf("Setting up a %dx%d %s console\n", mode->xres,
> +> > mode->yres, sunxi_get_mon_desc(sunxi_display.monitor));
Is it worth including the string "interlaced" here when appropriate?
(Ack stands either way)
Ian.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
