On Mon, Feb 15, 2021 at 8:19 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > On Mon, Feb 15, 2021 at 09:17:11PM +0900, Hector Martin wrote: > > + > > +/* Apple S5L */ > > +static int __init apple_s5l_early_console_setup(struct earlycon_device *device, > > + const char *opt) > > +{ > > + /* Close enough to S3C2410 for earlycon... */ > > + device->port.private_data = &s3c2410_early_console_data; > > + > > +#ifdef CONFIG_ARM64 > > if IS_ENABLED() > (unless it cannot be used due to missing symbol?) > > > + /* ... but we need to override the existing fixmap entry as nGnRnE */ > > + __set_fixmap(FIX_EARLYCON_MEM_BASE, device->port.mapbase, > > + __pgprot(PROT_DEVICE_nGnRnE)); > > +#endif It has to be a preprocessor conditional because PROT_DEVICE_nGnRnE is only defined on arm64. We could add a FIXMAP_PAGE_NONPOSTED alias for it that defaults to FIXMAP_PAGE_IO, but in the end this is really an architecture specific thing and I think leaving it guarded by the architecture is appropriate. > > + return samsung_early_console_setup(device, opt); > > Don't you need to handle the error code - set PROT_DEFAULT() or whatever > was there before? __set_fixmap() has no return value, it just writes a page table entry and does not fail. Arnd