Hi Heinrich,
Thanks for your review comments. See my reply below.
On Mon, Jul 16, 2018 at 07:52:20AM +0200, Heinrich Schuchardt wrote:
[--snip--]
> > diff --git a/include/efi.h b/include/efi.h
> > index 0fe15e65c06c..eb2a569fe010 100644
> > --- a/include/efi.h
> > +++ b/include/efi.h
> > @@ -162,20 +162,16 @@ enum efi_mem_type {
> > };
> >
> > /* Attribute values */
> > -enum {
> > - EFI_MEMORY_UC_SHIFT = 0, /* uncached */
> > - EFI_MEMORY_WC_SHIFT = 1, /* write-coalescing */
> > - EFI_MEMORY_WT_SHIFT = 2, /* write-through */
> > - EFI_MEMORY_WB_SHIFT = 3, /* write-back */
> > - EFI_MEMORY_UCE_SHIFT = 4, /* uncached, exported */
> > - EFI_MEMORY_WP_SHIFT = 12, /* write-protect */
> > - EFI_MEMORY_RP_SHIFT = 13, /* read-protect */
> > - EFI_MEMORY_XP_SHIFT = 14, /* execute-protect */
> > - EFI_MEMORY_RUNTIME_SHIFT = 63, /* range requires runtime mapping */
> > -
> > - EFI_MEMORY_RUNTIME = 1ULL << EFI_MEMORY_RUNTIME_SHIFT,
> > - EFI_MEM_DESC_VERSION = 1,
> > -};
> > +#define EFI_MEMORY_UC ((u64)0x0000000000000001ULL) /*
> > uncached */
>
> Unsigned long long int (ULL) is at least 64bit wide and unsigned (cf.
> https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html). Why do you introduce
> the (u64) conversion?
Because this is how it appears in Linux kernel:
$ git blame ./include/linux/efi.h | grep 'define EFI_MEMORY_.*u64'
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 90) #define
EFI_MEMORY_UC ((u64)0x0000000000000001ULL) /* uncached */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 91) #define
EFI_MEMORY_WC ((u64)0x0000000000000002ULL) /*
write-coalescing */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 92) #define
EFI_MEMORY_WT ((u64)0x0000000000000004ULL) /* write-through */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 93) #define
EFI_MEMORY_WB ((u64)0x0000000000000008ULL) /* write-back */
9c97e0bdd4b4a (Laszlo Ersek 2014-09-03 13:32:19 +0200 94) #define
EFI_MEMORY_UCE ((u64)0x0000000000000010ULL) /* uncached,
exported */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 95) #define
EFI_MEMORY_WP ((u64)0x0000000000001000ULL) /* write-protect */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 96) #define
EFI_MEMORY_RP ((u64)0x0000000000002000ULL) /* read-protect */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 97) #define
EFI_MEMORY_XP ((u64)0x0000000000004000ULL) /* execute-protect
*/
c016ca08f89c6 (Robert Elliott 2016-02-01 22:07:06 +0000 98) #define
EFI_MEMORY_NV ((u64)0x0000000000008000ULL) /* non-volatile */
87db73aebf555 (Ard Biesheuvel 2015-08-07 09:36:54 +0100 101) #define
EFI_MEMORY_RO ((u64)0x0000000000020000ULL) /* read-only */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 102) #define
EFI_MEMORY_RUNTIME ((u64)0x8000000000000000ULL) /* range requires runtime
mapping */
Unless we see a potential issue with that (and I don't see), IMO we
should stick to kernel sources as much as possible, since this makes
integration/re-sync process easier.
>
> Otherwise
>
> Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de>
>
> > +#define EFI_MEMORY_WC ((u64)0x0000000000000002ULL) /*
> > write-coalescing */
> > +#define EFI_MEMORY_WT ((u64)0x0000000000000004ULL) /*
> > write-through */
> > +#define EFI_MEMORY_WB ((u64)0x0000000000000008ULL) /*
> > write-back */
> > +#define EFI_MEMORY_UCE ((u64)0x0000000000000010ULL) /*
> > uncached, exported */
> > +#define EFI_MEMORY_WP ((u64)0x0000000000001000ULL) /*
> > write-protect */
> > +#define EFI_MEMORY_RP ((u64)0x0000000000002000ULL) /*
> > read-protect */
> > +#define EFI_MEMORY_XP ((u64)0x0000000000004000ULL) /*
> > execute-protect */
> > +#define EFI_MEMORY_RUNTIME ((u64)0x8000000000000000ULL) /* range
> > requires runtime mapping */
> > +#define EFI_MEM_DESC_VERSION 1
> >
> > #define EFI_PAGE_SHIFT 12
> > #define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT)
Best regards,
Eugeniu.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
