Message-ID: <CAK8P3a11Mk3XcmP5Ecbv55TuNRjrrp_43GX8hPDfdLh6JMHE=g@mail.gmail.com>
Date:   Wed, 8 Nov 2017 11:16:22 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Greentime Hu <green.hu@...il.com>
Cc:     greentime@...estech.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Networking <netdev@...r.kernel.org>,
        Vincent Chen <vincentc@...estech.com>
Subject: Re: [PATCH 26/31] nds32: Build infrastructure

On Wed, Nov 8, 2017 at 6:55 AM, Greentime Hu <green.hu@...il.com> wrote:

> diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
> new file mode 100644
> index 0000000..112f470
> --- /dev/null
> +++ b/arch/nds32/Kconfig
> @@ -0,0 +1,107 @@
> +#
> +# For a description of the syntax of this configuration file,
> +# see Documentation/kbuild/kconfig-language.txt.
> +#
> +
> +config NDS32
> +        def_bool y
> +       select ARCH_HAS_RAW_COPY_USER
> +       select ARCH_WANT_FRAME_POINTERS if FTRACE
> +       select ARCH_WANT_IPC_PARSE_VERSION
> +       select CLKSRC_MMIO
> +       select CLONE_BACKWARDS
> +       select TIMER_OF
> +       select FRAME_POINTER
> +       select GENERIC_ATOMIC64
> +       select GENERIC_CPU_DEVICES
> +       select GENERIC_CLOCKEVENTS
> +       select GENERIC_IOMAP
> +       select GENERIC_IRQ_CHIP
> +       select GENERIC_IRQ_PROBE
> +       select GENERIC_IRQ_SHOW
> +       select GENERIC_STRNCPY_FROM_USER
> +       select GENERIC_STRNLEN_USER
> +       select GENERIC_TIME_VSYSCALL
> +       select HAVE_ARCH_TRACEHOOK
> +       select HAVE_GENERIC_IOMAP

You normally don't want HAVE_GENERIC_IOMAP, at least unless the CPU
has special instructions to trigger PCI I/O port access.

> +       select HAVE_DEBUG_KMEMLEAK
> +       select HAVE_IDE

You certainly don't want HAVE_IDE

> +       select HAVE_MEMBLOCK
> +       select HAVE_MEMBLOCK_NODE_MAP
> +       select HAVE_UID16

HAVE_UID16 shouldn't be used on new architectures, as mentioned in the
comment about asm/posix_types.h

> +       select HAVE_REGS_AND_STACK_ACCESS_API
> +       select IRQ_DOMAIN
> +       select LOCKDEP_SUPPORT
> +       select MODULES_USE_ELF_REL
> +       select MODULES_USE_ELF_RELA

I would think that you use either MODULES_USE_ELF_REL or
MODULES_USE_ELF_RELA, but not both.

> +       select OF
> +       select OF_EARLY_FLATTREE
> +       select OLD_SIGACTION
> +       select OLD_SIGSUSPEND3

What are the OLD_SIG* ones for? It sounds like something you shouldn't
need, although I'm not familiar wiht them.

> +       select NO_IOPORT_MAP
> +       select RTC_LIB
> +       select THREAD_INFO_IN_TASK
> +       select SYS_SUPPORTS_APM_EMULATION

I don't see what SYS_SUPPORTS_APM_EMULATION gains you.

> +config GENERIC_CALIBRATE_DELAY
> +       def_bool y

It's better to avoid the delay loop completely and skip the calibration,
if your hardware allows.

> +
> +config NDS32_BUILTIN_DTB
> +        string "Builtin DTB"
> +        default ""
> +       help
> +         User can use it to specify the dts of the SoC

Better leave this up to the boot loader.

> +config ALIGNMENT_TRAP
> +       tristate "Kernel support unaligned access handling"
> +       default y
> +       help
> +         Andes processors cannot fetch/store information which is not
> +         naturally aligned on the bus, i.e., a 4 byte fetch must start at an
> +         address divisible by 4. On 32-bit Andes processors, these non-aligned
> +         fetch/store instructions will be emulated in software if you say
> +         here, which has a severe performance impact. This is necessary for
> +         correct operation of some network protocols. With an IP-only
> +         configuration it is safe to say N, otherwise say Y.

Which network protocols are you referring to?

> +config HIGHMEM
> +       bool "High Memory Support"
> +       depends on MMU && CPU_CACHE_NONALIASING
> +       help
> +         The address space of Andes processors is only 4 Gigabytes large
> +         and it has to accommodate user address space, kernel address
> +         space as well as some memory mapped IO. That means that, if you
> +         have a large amount of physical memory and/or IO, not all of the
> +         memory can be "permanently mapped" by the kernel. The physical
> +         memory that is not permanently mapped is called "high memory".
> +
> +         Depending on the selected kernel/user memory split, minimum
> +         vmalloc space and actual amount of RAM, you may not need this
> +         option which should result in a slightly faster kernel.
> +
> +         If unsure, say N.

Generally speaking, highmem support is a mess, and it's better to avoid it.

I see that the two device tree files you have list 1GB of memory. Do you think
that is a common configuration for actual products? Do you expect any to
have more than 1GB (or more than 4GB) in the future, or is that the upper
end of the scale?

If 1GB is a reasonable upper bound, then you could change the vmsplit
to give slightly less address space to user space and have 1GB of direct-mapped
kernel memory plus 256MB of vmalloc space reserved for the kernel,
and completely avoid highmem.

> +config MEMORY_START
> +       hex "Physical memory start address"
> +       default "0x00000000"
> +       help
> +         Physical memory start address, you may modify it if it is porting to
> +         a new SoC with different start address.
> +endmenu

On ARM, we found options like this to be rather problematic since it prevents
you from running the same kernel on boards that are otherwise compatible.

If the architecture easily allows the memory to start at address 0, could
you require this address for all SoCs that want to run Linux, and get
rid of the compile-time option?

       Arnd

Powered by blists - more mailing lists