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

Ard Biesheuvel
2019-12-11T18:06:33Z

On Wed, 11 Dec 2019 at 19:03, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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?