[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <479C5F69.1000707@monstr.eu>
Date: Sun, 27 Jan 2008 11:39:37 +0100
From: Michal Simek <monstr@...str.eu>
To: Randy Dunlap <randy.dunlap@...cle.com>
CC: linux-kernel@...r.kernel.org, stephen.neuendorffer@...inx.com,
john.williams@...alogix.com, microblaze-uclinux@...e.uq.edu.au,
sam@...nborg.org
Subject: Re: [PATCH 01/52] [microblaze] Kconfig patches
Hi Randy,
thanks for review. Comments are below.
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> new file mode 100644
>> index 0000000..6dbc069
>> --- /dev/null
>> +++ b/arch/microblaze/Kconfig
>> @@ -0,0 +1,160 @@
>> +# For a description of the syntax of this configuration file,
>> +# see Documentation/kbuild/config-language.txt.
>>
>
> It's kconfig-language.txt.
>
I fix it
>> +
>> +mainmenu "Linux/Microblaze Kernel Configuration"
>> +
>> +config MICROBLAZE
>> + bool
>> + default y
>>
>
> The one-line form:
> def_bool y
> is preferred, just to be more concise, use less screen real estate...
> Above and many times below.
>
fix.
>> +config MMU
>> + bool
>> + default n
>> +
>> +config SWAP
>> + bool
>> + default n
>> +
>> +config RWSEM_GENERIC_SPINLOCK
>> + bool
>> + default y
>> +
>> +config RWSEM_XCHGADD_ALGORITHM
>> + bool
>> +
>> +
>> +config ARCH_HAS_ILOG2_U32
>> + bool
>> + default n
>> +
>> +config ARCH_HAS_ILOG2_U64
>> + bool
>> + default n
>> +
>> +config GENERIC_FIND_NEXT_BIT
>> + bool
>> + default y
>> +
>> +config GENERIC_HWEIGHT
>> + bool
>> + default y
>> +
>> +config GENERIC_HARDIRQS
>> + bool
>> + default y
>> +
>> +config GENERIC_IRQ_PROBE
>> + bool
>> + default y
>> +
>> +config GENERIC_CALIBRATE_DELAY
>> + bool
>> + default y
>> +
>> +config PCI
>> + bool
>> + default n
>> +
>> +config UID16
>> + bool
>> + default y
>> +
>> +config DEFCONFIG_LIST
>> + string
>> + default "arch/$ARCH/defconfig"
>> +
>> +source "init/Kconfig"
>> +
>> +source "arch/microblaze/platform/Kconfig.platform"
>> +
>> +menu "Processor type and features"
>> +config PREEMPT
>> + bool "Preemptible Kernel"
>> + help
>> + This option reduces the latency of the kernel when reacting to
>> + real-time or interactive events by allowing a low priority process to
>> + be preempted even if it is in kernel mode executing a system call.
>> + This allows applications to run more reliably even when the system is
>> + under load.
>> +
>> + Say Y here if you are building a kernel for a desktop, embedded
>> + or real-time system. Say N if you are unsure.
>> +
>> +config PREEMPT_TIMES
>> + bool "Collect preemption latency times"
>> + depends on PREEMPT
>> + help
>> + Allow collection for preemption latency times.
>> +
>> +config XILINX_UNCACHED_SHADOW
>> + bool "Are you using uncached shadow for RAM ?"
>> + depends on MICROBLAZE
>> + default y
>> + help
>> + This is needed to be able to allocate uncachable memory regions.
>>
>
> We don't seem to spell uncacheable consistently.
>
> Kernel source tree:
> uncachable - 27 times
> uncacheable - 53 times
> google:
> uncachable - 11,900 hits
> uncacheable - 26,700 hits
>
> so I would use "uncacheable".
>
>
I don't know if is spelling correct. But I tried to find count of uncached
in kernel tree and a result is interesting. For me is not problem to
change it.
$ grep -rn "uncached" * | grep -v "microblaze" | grep "*" | wc -l
198
>> + The feature requires the design to define the RAM memory controller window
>> + to be twice as large as the actual physical memory.
>> +
>> +config LARGE_ALLOCS
>> + bool "Allow allocating large blocks (> 1MB) of memory"
>> + help
>> + Allow the slab memory allocator to keep chains for very large
>> + memory sizes - upto 32MB. You may need this if your system has
>>
>
> s/upto/up to/
>
fix.
>> + a lot of RAM, and you need to able to allocate very large
>> + contiguous chunks. If unsure, say N.
>>
>
> Please indent config symbol help text as indicated in
> Documentation/CodingStyle: one tab + 2 spaces for each line of help text.
> (Please check all help text.)
>
fix in all Kconfig files.
>> +comment "Boot options"
>> +
>> +config CMDLINE
>> + string "Default kernel command string"
>> + default ""
>> + help
>> + On some architectures there is currently no way for the boot loader
>> + to pass arguments to the kernel. For these architectures, you should
>> + supply some command-line options at build time by entering them
>> + here.
>> +
>> +config CMDLINE_FORCE
>> + bool "Force default kernel command string"
>> + help
>> + Set this to have arguments from the default kernel command string
>> + override those passed by the boot loader
>>
>
> End with period (".").
>
fix in all Kconfig files.
>> +
>> +config OF
>> + bool
>> + default y
>> + help
>> + Set this to turn on OF
>>
>
> End with period. Is this Open Firmware?
>
>
>> +config OF_DEVICE
>> + bool
>> + default y
>> + help
>> + Set this to turn on OF
>>
>
> Ditto.
>
Yes. Microblaze architecture use Open Firmware files. I set to this
value with def_boot y without help part.
>> diff --git a/arch/microblaze/Kconfig.debug b/arch/microblaze/Kconfig.debug
>> new file mode 100644
>> index 0000000..1ccdd84
>> --- /dev/null
>> +++ b/arch/microblaze/Kconfig.debug
>> @@ -0,0 +1,22 @@
>> +menu "Kernel hacking"
>> +
>> +source "lib/Kconfig.debug"
>> +
>> +config EARLY_PRINTK
>> + bool "EARLY_PRINTK"
>> + default y
>> +
>> +config EARLY_PRINTK_UARTLITE_ADDRESS
>> + hex "Physical address where UART Lite for early printk is mapped"
>> + depends on EARLY_PRINTK
>> + default "0x40600000"
>> + help
>> + Please enter physcal address where your uart lite is mapped
>>
>
> End with period. s/physcal/physical/
>
fix + all period faults.
>> +endchoice
>> +
>> +# This is stil a bit broken - disabling for now JW 20070504
>>
>
> s/stil/still/
>
fix
>> diff --git a/arch/microblaze/platform/generic/Kconfig.auto b/arch/microblaze/platform/generic/Kconfig.auto
>> new file mode 100644
>> index 0000000..751369b
>> --- /dev/null
>> +++ b/arch/microblaze/platform/generic/Kconfig.auto
>> @@ -0,0 +1,45 @@
>> +#
>> +# Platform Kconfig menu for Microblaze generic board
>> +#
>> +
>> +config KERNEL_BASE_ADDR
>> + hex "Physical address where Linux Kernel is"
>> + default "0x44000000"
>> + help
>> + BASE Address for kernel
>> +
>> +config XILINX_ERAM_SIZE
>> + hex "Size address of XILINX_RAM" if ALLOW_EDIT_AUTO && XILINX_UNCACHED_SHADOW
>>
>
> What is a "Size address"?
>
Size address is physical size of main memory - fix.
>> + default 0x02000000
>> +
>> +comment "Definitions for MICROBLAZE0"
>> + depends on ALLOW_EDIT_AUTO
>> +
>> +config XILINX_MICROBLAZE0_FAMILY
>> + string "Targetted FPGA family" if ALLOW_EDIT_AUTO
>> + default spartan3e
>> +
>> +config XILINX_MICROBLAZE0_HW_VER
>> + string "Core version number" if ALLOW_EDIT_AUTO
>> + default 5.00.c
>> +
>> +config XILINX_MICROBLAZE0_USE_MSR_INSTR
>> + int "USE_MSR_INSTR" if ALLOW_EDIT_AUTO
>> + default 1
>> +
>> +config XILINX_MICROBLAZE0_USE_BARREL
>> + int "USE_BARREL range (0:1)" if ALLOW_EDIT_AUTO
>> + default 1
>>
>
> These (above & below) look like booleans. If you really need a
> range, config symbols can be specified as ranges. See
> Documentation/kbuild/kconfig-language.txt.
>
Fix is with range property.
>> +config XILINX_MICROBLAZE0_USE_DIV
>> + int "USE_DIV range (0:1)" if ALLOW_EDIT_AUTO
>> + default 1
>> +
>> +config XILINX_MICROBLAZE0_USE_HW_MUL
>> + int "USE_HW_MUL range (0:1)" if ALLOW_EDIT_AUTO
>> + default 1
>> +
>> +config XILINX_MICROBLAZE0_USE_FPU
>> + int "USE_FPU range (0:1)" if ALLOW_EDIT_AUTO
>> + default 0
>> +
>> --
>>
>
> ---
> ~Randy
Regards,
Michal Simek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists