* Vivek Goyal [2008-06-25 18:43]: > > Thanks for the patch. Couple of thoughts. > > Do we really need another CONFIG option (CONFIG_FIRMWARE_MEMMAP)? To, > me this does not seem to be a big chunk of code at the same time I am > assuming that most of the people will use it (because of kexec). So > probably, it might not make lot of sense to put additional CONFIG option. Agreed. > [..] > > +#include <linux/string.h> > > +#include <linux/firmware-map.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/types.h> > > +#include <linux/bootmem.h> > > + > > +/* > > + * String representation of enum firmware_map_type from > > + * <linux/firmware-map.h>. > > + */ > > +const char *type_to_string_map[] = { > > + "RAM", /* MAP_RAM */ > > + "Reserved", /* MAP_RESERVED */ > > + "ACPI Tables", /* MAP_ACPI */ > > + "Non-volatile Storage", /* MAP_NVS */ > > +}; > > + > > How about leaving the decision of memory type on arch dependent code? How > about letting arch code pass you an string while adding entry and that > string will contain the type of memory. The way request_resource() is > implemented. Also agreed, see the resent patch. Bernhard -- Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development