From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Wed, 5 Aug 2015 17:13:39 +0200 Subject: [U-Boot] [PATCH 4/5] sunxi: display: Add support for interlaced modes In-Reply-To: <1438761956.18644.55.camel@hellion.org.uk> References: <1438638870-3728-1-git-send-email-hdegoede@redhat.com> <1438638870-3728-4-git-send-email-hdegoede@redhat.com> <1438761956.18644.55.camel@hellion.org.uk> Message-ID: <55C22823.3030609@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 05-08-15 10:05, Ian Campbell wrote: > 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. Agreed, fixed. > > 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) Also fixed. Thanks, Hans