From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Date: Wed, 05 Aug 2015 09:05:56 +0100 Subject: [U-Boot] [PATCH 4/5] sunxi: display: Add support for interlaced modes In-Reply-To: <1438638870-3728-4-git-send-email-hdegoede@redhat.com> References: <1438638870-3728-1-git-send-email-hdegoede@redhat.com> <1438638870-3728-4-git-send-email-hdegoede@redhat.com> Message-ID: <1438761956.18644.55.camel@hellion.org.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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.