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