Re: [PATCH] efi/earlycon: Fix write-combine mapping on x86

Andy Shevchenko
2019-12-11T18:03:33Z

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