On Friday 06 December 2013, Leif Lindholm wrote:
>
> On Fri, Dec 06, 2013 at 02:59:48AM +0100, Arnd Bergmann wrote:
> > On Thursday 28 November 2013, Leif Lindholm wrote:
> > > @@ -898,6 +900,10 @@ void __init setup_arch(char **cmdline_p)
> > > sanity_check_meminfo();
> > > arm_memblock_init(&meminfo, mdesc);
> > >
> > > +#ifdef CONFIG_EFI
> > > + uefi_memblock_arm_reserve_range();
> > > +#endif
> > > +
> >
> > Better use
> >
> > if (IS_ENABLED(CONFIG_EFI))
> >
> > here for readability and better build-time checking of the interface when
> > CONFIG_EFI is disabled.
>
> I think I'll do what Grant suggested and stub it out in asm/uefi.h.
That's ok, too. I usually prefer the IS_ENABLED() method if there is only
one caller, since it's harder to get wrong, but the result is similar.
> > > +/*
> > > + * Returns 1 if 'facility' is enabled, 0 otherwise.
> > > + */
> > > +int efi_enabled(int facility)
> > > +{
> > > + return test_bit(facility, &arm_uefi_facility) != 0;
> > > +}
> > > +EXPORT_SYMBOL(efi_enabled);
> >
> > I'd use EXPORT_SYMBOL_GPL() unless there is a documented reason why
> > a symbol should be available to GPL-incompatible modules.
>
> So ... this is duplicating x86 code which Matt Fleming has promised
> to make generic. I'd prefer to keep it identical to x86 until this
> copy goes away.
Ok, makes sense.
> > > +static int __init remap_region(efi_memory_desc_t *md, efi_memory_desc_t *entry)
> > > +{
> > > + u64 va;
> > > + u64 paddr;
> > > + u64 size;
> > > +
> > > + *entry = *md;
> > > + paddr = entry->phys_addr;
> > > + size = entry->num_pages << EFI_PAGE_SHIFT;
> > > +
> > > + /*
> > > + * Map everything writeback-capable as coherent memory,
> > > + * anything else as device.
> > > + */
> > > + if (md->attribute & EFI_MEMORY_WB)
> > > + va = (u64)((u32)uefi_remap(paddr, size) & 0xffffffffUL);
> > > + else
> > > + va = (u64)((u32)uefi_ioremap(paddr, size) & 0xffffffffUL);
> >
> > Same here. Why is 'va' a u64?
>
> Because the field in the structure (defined by UEFI) is 64-bit:
>
> entry->virt_addr = va;
I see. Maybe put the typecast in that final assignment then?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
