On Thu, Jun 26, 2008 at 10:13:23AM +0200, Mikael Pettersson wrote: > Vivek Goyal writes: > > On Wed, Jun 25, 2008 at 09:57:05PM +0200, Bernhard Walle wrote: > > > This patch adds /sys/firmware/memmap interface that represents the BIOS > > > (or Firmware) provided memory map. The tree looks like: > > > > > > /sys/firmware/memmap/0/start (hex number) > > > end (hex number) > > > type (string) > > > ... /1/start > > > end > > > type > > > > > > With the following shell snippet one can print the memory map in the same form > > > the kernel prints itself when booting on x86 (the E820 map). > > > > > > --------- 8< -------------------------- > > > #!/bin/sh > > > cd /sys/firmware/memmap > > > for dir in * ; do > > > start=$(cat $dir/start) > > > end=$(cat $dir/end) > > > type=$(cat $dir/type) > > > printf "%016x-%016x (%s)\n" $start $[ $end +1] "$type" > > > done > > > --------- >8 -------------------------- > > > > > > That patch only provides the needed interface: > > > > > > 1. The sysfs interface. > > > 2. The structure and enumeration definition. > > > 3. The function firmware_map_add() and firmware_map_add_early() > > > that should be called from architecture code (E820/EFI, for > > > example) to add the contents to the interface. > > > > > > If the kernel is compiled without CONFIG_FIRMWARE_MEMMAP, the interface does > > > nothing without cluttering the architecture-specific code with #ifdef's. > > > > > > > Hi Bernhard, > > > > 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 > > It should be configurable. Whether it's done via CONFIG_KEXEC or its own > option I don't care. > That's fine. I am just curious how does one decide whether a particular functionality should have a separate CONFIG option or not. I thought if a functionality is sufficiently big/significant (in terms of code and in terms of memory allocation etc), then it is a good idea to put it under a CONFIG option, in case people want to compile it out. Just that looking at the patch, personally, I don't think that it makes lot of sense to create a separate CONFIG option for this patch. Thanks Vivek
