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. -- With Best Regards, Andy Shevchenko
