On Wed, 11 Dec 2019 at 19:03, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Wed, Dec 11, 2019 at 12:37:46PM -0500, Arvind Sankar wrote: > > On Wed, Dec 11, 2019 at 01:04:35PM +0200, Andy Shevchenko wrote: > > > > > > Make sense. > > > One comment below. > > > > > > > > - if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL)) > > > > > - efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WB); > > > > > - else > > > > > - efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WC); > > > > > + efi_fb = memremap(fb_base, screen_info.lfb_size, > > > > > + fb_wb ? MEMREMAP_WB : MEMREMAP_WC); > > > > > > I would really like to keep the style with if-else. > > > > > I edited this back to the if/else and then realized why I chose the > > ternary. It makes it easier for the reader to see that the only thing > > that depends on fb_wb is the MEMREMAP_ flag that gets used, while with > > the if/else the reader needs to compare both function invocations to see > > that that's the only difference. > > > > It's not a big deal, so if you still prefer the if/else I'll revise the > > patch. > > Perhaps comment near to if can explain this. > I'm fine with the ternary, actually. What do you feel is wrong with it?