Please download second patches for review:
http://mprc.pku.edu.cn/~guanxuetao/linux/patch-2.6.37-rc2-uc32-gxt2-arch.bz2
(above patch for arch/unicore32 directory, 149KB)
http://mprc.pku.edu.cn/~guanxuetao/linux/patch-2.6.37-rc2-uc32-gxt2-drivers.
bz2
(above patch for drivers/staging/puv3 directory, which including some
necessary drivers, 31KB)
http://mprc.pku.edu.cn/~guanxuetao/linux/patch-2.6.37-rc2-uc32-gxt2-rest.bz2
(above patch for rest patch code, for modifying the file not in
arch/unicore32 and drivers/staging/puv3, 2KB)
Replying Arnd's:
> It would be good if you could make a tool chain available, ideally both
> a patch against gcc and pre-built binaries for an x86-unicore cross
> compiler. This is not strictly required, but it helps to get people
> to do build tests on your code.
[Guan] please download x86-unicore cross toolchain from:
http://mprc.pku.edu.cn/~guanxuetao/linux/uc4-1.0.5-hard.tgz
(about 100MB)
It should be un-tar to /usr/unicore/gnu-toolchain-unicore directory.
> Further, you should find a way to publish git trees with your code, since
> a patch on a web site is less than ideal for transporting code changes,
> especially once the code gets merged into linux-next and mainline
afterwards.
> If your university or company has problems setting up an official git
> server, you can ask for an account on kernel.org, see
> http://www.kernel.org/faq/#account
>
[Guan] I have requested for kernel.org account, but no response now.
> Detailed comments below.
>
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/boot/compressed/decompress.c
> linux-2.6.37.y/arch/unicore32/boot/compressed/decompress.c
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/boot/compressed/misc.c
> linux-2.6.37.y/arch/unicore32/boot/compressed/misc.c
>
> These two files look very generic. It may be a good time to now move
> them into the global lib/ directory and make them usable by all
> architectures that do not require special fixups in the decompress
> code.
[Guan] I have modified these files, and make it more generic.
> > +MKIMAGE := $(srctree)/scripts/mkuboot.sh
>
> Do you have u-boot support as well?
[Guan] Yes, we use u-boot as bootloader.
> > +CONFIG_CMDLINE_FROM_U_BOOT=y
> > +CONFIG_CMDLINE="mem=512M ignore_loglevel
> video=unifb:1024x600-16@75 root=/dev/nfs rw
> nfsroot=/home/nfs/uc3,rsize=1024,wsize=1024
> ip=192.168.10.83:192.168.10.82:192.168.10.1:255.255.255.0::eth0:off"
>
> You should never need to pass the memory size in the command line.
> Please ensure that the boot loader always passes the memory size
> using your boot protocol (ideally in a device tree).
>
[Guan] CMDLINE will only be used when no bootloader params.
And u-boot params have another cmdline to replace this one.
> Similarly, the frame buffer size should ideally be autodetected
> using hardware or the boot protocol, though that can be harder.
[Guan] I have to remain this method at present.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/configs/nb0916_defconfig
> linux-2.6.37.y/arch/unicore32/configs/nb0916_defconfig
> > --- linux-2.6.37-rc1/arch/unicore32/configs/nb0916_defconfig
1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/configs/nb0916_defconfig 2010-11-08
> 10:14:07.110692045 +0800
>
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/configs/smw0919_defconfig
> linux-2.6.37.y/arch/unicore32/configs/smw0919_defconfig
> > --- linux-2.6.37-rc1/arch/unicore32/configs/smw0919_defconfig
> 1970-01-01 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/configs/smw0919_defconfig
> 2010-11-08 10:14:07.110692045 +0800
>
> The files look mostly identical. If there is no fundamental difference
> between them, I would recommend providing only a single defconfig file
> that works on all the machines.
[Guan] nb0916 is for netbook and desktop, and CONFIG_MODULES must be
enabled.
Smw0919 is for safety embedded devices, only necessary devices/drivers are
integrated.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h
> linux-2.6.37.y/arch/unicore32/include/asm/dma.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/dma.h 2010-11-11
> 09:55:29.517572760 +0800
> > @@ -0,0 +1,34 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/dma.h
>
> Just use the asm-generic file.
[Guan] when I set MAX_DMA_ADDRESS to PAGE_OFFSET, some pci drivers have
problem.
I will check it later.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/fixmap.h
> linux-2.6.37.y/arch/unicore32/include/asm/fixmap.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/fixmap.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/fixmap.h 2010-11-08
> 15:36:42.574862020 +0800
> > @@ -0,0 +1,51 @@
> > +/*
> > + * Nothing too fancy for now.
> > + *
> > + * On UniCore we already have well known fixed virtual addresses
imposed
> by
> > + * the architecture such as the vector page which is located at
0xffff0000,
>
> The comment is copied from ARM, but is it really true on unicore?
>
[Guan] Yes.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h
> linux-2.6.37.y/arch/unicore32/include/asm/highmem.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/highmem.h 2010-11-08
> 15:53:09.077836027 +0800
> > @@ -0,0 +1,53 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/highmem.h
>
> What is the maximum amount of memory you support on the 32 bit machines?
> We're removing all the optimizations for highmem from the kernel now, so
> I would recommend not to support this in new architectures if you can
> avoid it.
[Guan] we need to support 2GB memory.
>
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/io.h
> linux-2.6.37.y/arch/unicore32/include/asm/io.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/io.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/io.h 2010-11-11
> 10:23:14.258566201 +0800
> > @@ -0,0 +1,275 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/io.h
>
> This also looks similar to the asm-generic version. Can you use that, or
> possibly change it enough so you can?
[Guan] I recommend splitting asm-generic/io.h into io.h and ioremap.h
> > +/*
> > + * Use this value to indicate lack of interrupt
> > + * capability
> > + */
> > +#ifndef NO_IRQ
> > +#define NO_IRQ ((unsigned int)(-1))
> > +#endif
>
> This should not be required at all any more.
>
[Guan] amba bus driver need NO_IRQ
> > +#####
> > +# Auto Generate the files that only include the corresponding
asm-generic
> file
> > +# 6 files in 27-uc: errno.h fcntl.h ioctl.h poll.h resource.h
siginfo.h
> > +
> > +define cmd_asmgeneric
> > + (set -e; \
> > + echo '#include <asm-generic/$(notdir $@)>' ) > $@
> > +endef
>
> Nice trick. I'd love to have something like this in the common code so
> we can do the same for all architectures.
>
> Does this work with external object directories, i.e. building with
> make O=objdir, and with make headers_install?
>
[Guan] For UniCore, more than forty headers are auto generated.
However, if all non-exist headers are auto-generated, it would make a
terrible mess.
So, it should depend on archprepare rule in arch/xxx/Makefile, but not in
Kbuild.
I think the arch maintainers should determine which files are needed.
Also, make headers_install works well.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h
> linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h 2010-11-08
> 17:46:49.475569661 +0800
> > @@ -0,0 +1,65 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/mach/pci.h
> > +
> > +struct hw_pci {
> > +#ifdef CONFIG_PCI_DOMAINS
> > + int domain;
> > +#endif
> > + struct list_head buses;
> > + int nr_controllers;
> > + int (*setup)(int nr, struct pci_sys_data *);
> > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *);
> > + void (*preinit)(void);
> > + void (*postinit)(void);
> > + u8 (*swizzle)(struct pci_dev *dev, u8 *pin);
> > + int (*map_irq)(struct pci_dev *dev, u8 slot, u8 pin);
> > +};
> > +
> > +/*
> > + * Per-controller structure
> > + */
> > +struct pci_sys_data {
> > +#ifdef CONFIG_PCI_DOMAINS
> > + int domain;
> > +#endif
> > + struct list_head node;
> > + int busnr; /* primary bus number */
> > + u64 mem_offset; /* bus->cpu memory mapping offset */
> > + unsigned long io_offset; /* bus->cpu IO mapping offset */
> > + struct pci_bus *bus; /* PCI bus */
> > + struct resource *resource[3]; /* Primary PCI bus resources */
> > + /* Bridge swizzling */
> > + u8 (*swizzle)(struct pci_dev *, u8 *);
> > + /* IRQ mapping */
> > + int (*map_irq)(struct pci_dev *, u8, u8);
> > + struct hw_pci *hw;
> > + void *private_data; /* platform controller private data
*/
> > +};
>
> These are two separate abstractions for multiple PCI controllers,
> but your code does not even contain one implementation.
>
> I can only assume that you actually have a PCI implementation, but
> if you do not have more than one, you are better off implementing just
> the one you have instead of extra wrappers.
[Guan] we do have pci bus and pci driver. I have to implement these
interface for pci compatibility.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h
> linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h 2010-11-08
> 17:48:06.111456784 +0800
> > @@ -0,0 +1,52 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/mach/time.h
>
> Can be removed when you get rid of the machine_desc abstraction.
[Guan] sys_timer can't be removed, which is used for device register.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h
> linux-2.6.37.y/arch/unicore32/include/asm/memblock.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/memblock.h 2010-11-10
> 18:59:14.326006751 +0800
> > @@ -0,0 +1,22 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/memblock.h
>
> This too.
[Guan] we use memblock as infrastructure, and perhaps it's not good.
> Also, do you actually support both MMU and NOMMU modes?
[Guan] Only support MMU mode.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/swab.h
> linux-2.6.37.y/arch/unicore32/include/asm/swab.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/swab.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/swab.h 2010-11-09
> 09:28:21.005577540 +0800
> > @@ -0,0 +1,51 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/swab.h
> > + *
> > + * Code specific to PKUnity SoC and UniCore ISA
> > + *
> > + * Copyright (C) 2001-2008 GUAN Xue-tao
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * In little endian mode, the data bus is connected such
> > + * that byte accesses appear as:
> > + * 0 = d0...d7, 1 = d8...d15, 2 = d16...d23, 3 = d24...d31
> > + * and word accesses (data or instruction) appear as:
> > + * d0...d31
> > + */
> > +#ifndef __UNICORE_SWAB_H__
> > +#define __UNICORE_SWAB_H__
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/types.h>
> > +
> > +#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> > +# define __SWAB_64_THRU_32__
> > +#endif
> > +
> > +static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
>
> Can't you use the __builtin_bswap32/__builtin_bswap64 provided by gcc?
[Guan] __bswapsi2 is in libgcc, which is needed by __builtin_bswap32.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h
> linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h 2010-11-09
> 11:33:30.209566466 +0800
> > @@ -0,0 +1,408 @@
> > +/*
> > + * linux/arch/unicore32/include/asm/uaccess.h
>
> Please have another look at the asm-generic version of this file.
> With the current version it may be useful to base on that.
[Guan] __copy_from_user and __copy_to_user is too expensive for 1/2/4 words.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h
> linux-2.6.37.y/arch/unicore32/include/asm/unistd.h
> > --- linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h 1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.37.y/arch/unicore32/include/asm/unistd.h 2010-11-11
> 10:47:36.743710466 +0800
>
> Please use the cleaned-up asm-generic version as arch/tile does now.
> You add a lot of overhead otherwise.
[Guan] we use __NR_* as the immediate number in soft-interrupt instruction.
And if the syscall number changed, all software and toolchain need
re-compiled.
> > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> linux-2.6.37-rc1/arch/unicore32/Kconfig
linux-2.6.37.y/arch/unicore32/Kconfig
> > --- linux-2.6.37-rc1/arch/unicore32/Kconfig 1970-01-01
08:00:00.000000000
> +0800
> > +++ linux-2.6.37.y/arch/unicore32/Kconfig 2010-11-11
10:40:02.001225028
> +0800
> > @@ -0,0 +1,337 @@
> > + config ARCH_PUV3
> > + bool "PKUnity v3 (SuperK)"
> > + select CPU_UCV2
> > + select PKUNITY_AMBA
> > + select ZONE_DMA
> > + select EMBEDDED
> > + select GENERIC_CLOCKEVENTS
> > + select HAVE_CLK
> > + select ARCH_USES_GETTIMEOFFSET
> > + select ARCH_REQUIRE_GPIOLIB
> > + select ARCH_HAS_CPUFREQ
>
> You might not want to select ZONE_DMA and ARCH_USES_GETTIMEOFFSET,
> since you are generally better off without them.
[Guan] We need ZONE_DMA for only 128M low memory could be used for DMA.
> That's all for now, let's discuss my comments first and then I'll
> start commenting on the next parts.
[Guan] Thank you very much!
Guan Xuetao
>
> > > +#####
> > > +# Auto Generate the files that only include the corresponding
> asm-generic
> > file
> > > +# 6 files in 27-uc: errno.h fcntl.h ioctl.h poll.h resource.h
> siginfo.h
> > > +
> > > +define cmd_asmgeneric
> > > + (set -e; \
> > > + echo '#include <asm-generic/$(notdir $@)>' ) > $@
> > > +endef
> >
> > Nice trick. I'd love to have something like this in the common code so
> > we can do the same for all architectures.
Just a quick proof-of-concept hack.
Sam
diff --git a/Makefile b/Makefile
index ab5359d..e878120 100644
--- a/Makefile
+++ b/Makefile
@@ -344,7 +344,9 @@ CFLAGS_GCOV = -fprofile-arcs -ftest-coverage
# Use LINUXINCLUDE when you must reference the include/ directory.
# Needed to be compatible with the O= option
-LINUXINCLUDE := -I$(srctree)/arch/$(hdr-arch)/include -Iinclude \
+LINUXINCLUDE := -I$(srctree)/arch/$(hdr-arch)/include \
+ $(if $(KBUILD_SRC), -Iarch/$(hdr-arch)/include) \
+ -Iinclude \
$(if $(KBUILD_SRC), -I$(srctree)/include) \
-include include/generated/autoconf.h
@@ -950,7 +952,13 @@ prepare1: prepare2 include/linux/version.h include/generated/utsrelease.h \
archprepare: prepare1 scripts_basic
-prepare0: archprepare FORCE
+archheaders: $(addprefix arch/$(SRCARCH)/include/asm/, $(ARCH_GENERIC_HEADERS))
+
+$(addprefix arch/$(SRCARCH)/include/asm/, $(ARCH_GENERIC_HEADERS)): arch/$(SRCARCH)/Makefile
+ $(Q)mkdir -p $(dir $@)
+ $(Q)echo '#include <asm-generic/$(notdir $@)>' > $@
+
+prepare0: archprepare archheaders FORCE
$(Q)$(MAKE) $(build)=.
$(Q)$(MAKE) $(build)=. missing-syscalls
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index b02e509..fd23d01 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -12,6 +12,8 @@ endif
# e.g.: obj-y += foo_$(BITS).o
export BITS
+ARCH_GENERIC_HEADERS := termios.h
+
ifeq ($(CONFIG_X86_32),y)
BITS := 32
UTS_MACHINE := i386
diff --git a/arch/x86/include/asm/termios.h b/arch/x86/include/asm/termios.h
deleted file mode 100644
index 280d78a..0000000
--- a/arch/x86/include/asm/termios.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <asm-generic/termios.h>
On Saturday 20 November 2010, Guan wrote:
> Please download second patches for review:
> Replying Arnd's:
> > It would be good if you could make a tool chain available, ideally both
> > a patch against gcc and pre-built binaries for an x86-unicore cross
> > compiler. This is not strictly required, but it helps to get people
> > to do build tests on your code.
> [Guan] please download x86-unicore cross toolchain from:
> http://mprc.pku.edu.cn/~guanxuetao/linux/uc4-1.0.5-hard.tgz
> (about 100MB)
> It should be un-tar to /usr/unicore/gnu-toolchain-unicore directory.
Ok, thanks for making this available.
> > > +CONFIG_CMDLINE_FROM_U_BOOT=y
> > > +CONFIG_CMDLINE="mem=512M ignore_loglevel
> > video=unifb:1024x600-16@75 root=/dev/nfs rw
> > nfsroot=/home/nfs/uc3,rsize=1024,wsize=1024
> > ip=192.168.10.83:192.168.10.82:192.168.10.1:255.255.255.0::eth0:off"
> >
> > You should never need to pass the memory size in the command line.
> > Please ensure that the boot loader always passes the memory size
> > using your boot protocol (ideally in a device tree).
> >
> [Guan] CMDLINE will only be used when no bootloader params.
> And u-boot params have another cmdline to replace this one.
Well, my point was more specifically that having to provide
options on the command line that are hardware specific is wrong,
independent of whether they come from the hardcoded command line
or from u-boot.
The command line can of course be used for boot device configuration
like the NFS root stuff (which I still would not put in the defconfig),
but memory size and screen resolution are something that should
always be detectable these days.
> > The files look mostly identical. If there is no fundamental difference
> > between them, I would recommend providing only a single defconfig file
> > that works on all the machines.
> [Guan] nb0916 is for netbook and desktop, and CONFIG_MODULES must be
> enabled.
> Smw0919 is for safety embedded devices, only necessary devices/drivers are
> integrated.
The defconfig is not typically used without modifications on real systems,
but serves as a starting point. If it's reasonable to build a configuration
with all the smw0919 device drivers built-in and the additional nb0916
drivers as loadable modules, I guess that would make a reasonable defconfig.
On smw0919 hardware, you can then just choose not to install the modules into
the root file system.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h
> > linux-2.6.37.y/arch/unicore32/include/asm/dma.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/dma.h 2010-11-11
> > 09:55:29.517572760 +0800
> > > @@ -0,0 +1,34 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/dma.h
> >
> > Just use the asm-generic file.
> [Guan] when I set MAX_DMA_ADDRESS to PAGE_OFFSET, some pci drivers have
> problem.
> I will check it later.
Interesting. You already allow to override MAX_DMA_ADDRESS to
(PAGE_OFFSET + SZ_128M) when PCI is enabled. It would be good to
find out (and document!) the signficance of the extra 128MB here.
If it turns out to be required, please make a patch to the
asm-generic/dma.h file to allow overriding MAX_DMA_ADDRESS in the
same way you do in your version.
> > > + * Nothing too fancy for now.
> > > + *
> > > + * On UniCore we already have well known fixed virtual addresses imposed by
> > > + * the architecture such as the vector page which is located at 0xffff0000,
> >
> > The comment is copied from ARM, but is it really true on unicore?
> >
> [Guan] Yes.
In what way does the architecture enforce this? What are the contents
of this page? Can you make it an actual VDSO rather than a magic page
that sits in the user address space?
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h
> > linux-2.6.37.y/arch/unicore32/include/asm/highmem.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/highmem.h 2010-11-08
> > 15:53:09.077836027 +0800
> > > @@ -0,0 +1,53 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/highmem.h
> >
> > What is the maximum amount of memory you support on the 32 bit machines?
> > We're removing all the optimizations for highmem from the kernel now, so
> > I would recommend not to support this in new architectures if you can
> > avoid it.
> [Guan] we need to support 2GB memory.
Would it be an option to have a fixed 2GB/2GB split between user and physical
address space? That would limit the user addressable range to something
just under 2GB (because of vmalloc/ioremap pages), but would significantly
simplify and speed up user memory access from kernel space.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/io.h
> > linux-2.6.37.y/arch/unicore32/include/asm/io.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/io.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/io.h 2010-11-11
> > 10:23:14.258566201 +0800
> > > @@ -0,0 +1,275 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/io.h
> >
> > This also looks similar to the asm-generic version. Can you use that, or
> > possibly change it enough so you can?
> [Guan] I recommend splitting asm-generic/io.h into io.h and ioremap.h
Good idea. The ioremap parts of io.h are really specific to nommu
architectures, while the other parts are specific to architectures that
do not require specific ordering instructions.
> > > +/*
> > > + * Use this value to indicate lack of interrupt
> > > + * capability
> > > + */
> > > +#ifndef NO_IRQ
> > > +#define NO_IRQ ((unsigned int)(-1))
> > > +#endif
> >
> > This should not be required at all any more.
> >
> [Guan] amba bus driver need NO_IRQ
That is a bug, as far as I can tell. I think it should be changed to
a hardcoded (-1) in the amba drivers as we do elsewhere.
Until now, I was not even aware that we had an amba bus driver,
and I'm not conviced that it's a good idea to use it either
in its current form.
Most of the users of the amba bus currently declare static devices
in the architecture tree, which is something that breaks a number
of assumptions in the device model about the life time of device
structures.
> > > +define cmd_asmgeneric
> > > + (set -e; \
> > > + echo '#include <asm-generic/$(notdir $@)>' ) > $@
> > > +endef
> >
> > Nice trick. I'd love to have something like this in the common code so
> > we can do the same for all architectures.
> >
> > Does this work with external object directories, i.e. building with
> > make O=objdir, and with make headers_install?
> >
> [Guan] For UniCore, more than forty headers are auto generated.
> However, if all non-exist headers are auto-generated, it would make a
> terrible mess.
> So, it should depend on archprepare rule in arch/xxx/Makefile, but not in
> Kbuild.
> I think the arch maintainers should determine which files are needed.
Keeping the list in the architecture maintainer's hands sounds right.
arch/xxx/Makefile however is really overloaded already, it contains
all sorts of ad-hoc Makefile code, while a list of files to be generated
would be more structured.
> Also, make headers_install works well.
Good.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h
> > linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h 2010-11-08
> > 17:46:49.475569661 +0800
> > > @@ -0,0 +1,65 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/mach/pci.h
> > > +
> > > +struct hw_pci {
> > > +#ifdef CONFIG_PCI_DOMAINS
> > > + int domain;
> > > +#endif
> > > + struct list_head buses;
> > > + int nr_controllers;
> > > + int (*setup)(int nr, struct pci_sys_data *);
> > > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *);
> > > + void (*preinit)(void);
> > > + void (*postinit)(void);
> > > + u8 (*swizzle)(struct pci_dev *dev, u8 *pin);
> > > + int (*map_irq)(struct pci_dev *dev, u8 slot, u8 pin);
> > > +};
> > > +
> > > +/*
> > > + * Per-controller structure
> > > + */
> > > +struct pci_sys_data {
> > > +#ifdef CONFIG_PCI_DOMAINS
> > > + int domain;
> > > +#endif
> > > + struct list_head node;
> > > + int busnr; /* primary bus number */
> > > + u64 mem_offset; /* bus->cpu memory mapping offset */
> > > + unsigned long io_offset; /* bus->cpu IO mapping offset */
> > > + struct pci_bus *bus; /* PCI bus */
> > > + struct resource *resource[3]; /* Primary PCI bus resources */
> > > + /* Bridge swizzling */
> > > + u8 (*swizzle)(struct pci_dev *, u8 *);
> > > + /* IRQ mapping */
> > > + int (*map_irq)(struct pci_dev *, u8, u8);
> > > + struct hw_pci *hw;
> > > + void *private_data; /* platform controller private data
> */
> > > +};
> >
> > These are two separate abstractions for multiple PCI controllers,
> > but your code does not even contain one implementation.
> >
> > I can only assume that you actually have a PCI implementation, but
> > if you do not have more than one, you are better off implementing just
> > the one you have instead of extra wrappers.
> [Guan] we do have pci bus and pci driver. I have to implement these
> interface for pci compatibility.
Ok. I would put the actual PCI implementation into the architecture
patch series then, but get rid of the indirect function pointers.
Do you support multiple PCI domains (root bridges)?
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h
> > linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h 2010-11-08
> > 17:48:06.111456784 +0800
> > > @@ -0,0 +1,52 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/mach/time.h
> >
> > Can be removed when you get rid of the machine_desc abstraction.
> [Guan] sys_timer can't be removed, which is used for device register.
I don't understand. I can only see one sys_timer instance, the puv3_timer,
so you can just as well hardcode access to that.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h
> > linux-2.6.37.y/arch/unicore32/include/asm/memblock.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/memblock.h 2010-11-10
> > 18:59:14.326006751 +0800
> > > @@ -0,0 +1,22 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/memblock.h
> >
> > This too.
> [Guan] we use memblock as infrastructure, and perhaps it's not good.
No, there is nothing wrong with memblock.
My point was that the indirect function calls through machine_desc that
you use here are not needed.
I would also recommend using only memblock and not also bootmem, by
unconditionally setting CONFIG_NO_BOOTMEM.
> > Also, do you actually support both MMU and NOMMU modes?
> [Guan] Only support MMU mode.
ok
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/swab.h
> > linux-2.6.37.y/arch/unicore32/include/asm/swab.h
> > > +#ifndef __UNICORE_SWAB_H__
> > > +#define __UNICORE_SWAB_H__
> > > +
> > > +#include <linux/compiler.h>
> > > +#include <linux/types.h>
> > > +
> > > +#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> > > +# define __SWAB_64_THRU_32__
> > > +#endif
> > > +
> > > +static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
> >
> > Can't you use the __builtin_bswap32/__builtin_bswap64 provided by gcc?
> [Guan] __bswapsi2 is in libgcc, which is needed by __builtin_bswap32.
You could link against libgcc, see arch/tile/Makefile for an example
how to do that.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h
> > linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h 2010-11-09
> > 11:33:30.209566466 +0800
> > > @@ -0,0 +1,408 @@
> > > +/*
> > > + * linux/arch/unicore32/include/asm/uaccess.h
> >
> > Please have another look at the asm-generic version of this file.
> > With the current version it may be useful to base on that.
> [Guan] __copy_from_user and __copy_to_user is too expensive for 1/2/4 words.
The asm-generic version of __copy_from_user/__copy_to_user uses
__builtin_constant_p() to optimize away all this overhead at build time,
so even a copy_to_user(dst, src, 4) gets turned into a very small number
of instructions.
This should work for both cases: when called with a constant size
and when called from get_user/put_user.
I suppose you can do the same thing in your code.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h
> > linux-2.6.37.y/arch/unicore32/include/asm/unistd.h
> > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h 1970-01-01
> > 08:00:00.000000000 +0800
> > > +++ linux-2.6.37.y/arch/unicore32/include/asm/unistd.h 2010-11-11
> > 10:47:36.743710466 +0800
> >
> > Please use the cleaned-up asm-generic version as arch/tile does now.
> > You add a lot of overhead otherwise.
> [Guan] we use __NR_* as the immediate number in soft-interrupt instruction.
> And if the syscall number changed, all software and toolchain need
> re-compiled.
Yes, I realize that. I think it would be very hard to integrate your
architecture code without breaking compatibility, but breaking it once
in order to fix all the problems arising from your backwards-compatibility
code will make your life much easier in the long run.
One possible way to do the migration would be to do the upstream
integration using only the new ABI, but keep a private patch that
reverts to the existing ABI. At some point in the future, you can
then declare the old ABI obsolete and drop the patch, supporting
only the new ABI.
> > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > linux-2.6.37-rc1/arch/unicore32/Kconfig
> linux-2.6.37.y/arch/unicore32/Kconfig
> > > --- linux-2.6.37-rc1/arch/unicore32/Kconfig 1970-01-01
> 08:00:00.000000000
> > +0800
> > > +++ linux-2.6.37.y/arch/unicore32/Kconfig 2010-11-11
> 10:40:02.001225028
> > +0800
> > > @@ -0,0 +1,337 @@
> > > + config ARCH_PUV3
> > > + bool "PKUnity v3 (SuperK)"
> > > + select CPU_UCV2
> > > + select PKUNITY_AMBA
> > > + select ZONE_DMA
> > > + select EMBEDDED
> > > + select GENERIC_CLOCKEVENTS
> > > + select HAVE_CLK
> > > + select ARCH_USES_GETTIMEOFFSET
> > > + select ARCH_REQUIRE_GPIOLIB
> > > + select ARCH_HAS_CPUFREQ
> >
> > You might not want to select ZONE_DMA and ARCH_USES_GETTIMEOFFSET,
> > since you are generally better off without them.
> [Guan] We need ZONE_DMA for only 128M low memory could be used for DMA.
Ok. Is this only for some device drivers, or is this a general limitation?
Arnd
> Subject: Re: [PATCH] a new UniCore32 arch-dependent patch for linux-
> 2.6.37-rc1
> > > > +CONFIG_CMDLINE_FROM_U_BOOT=y
> > > > +CONFIG_CMDLINE="mem=512M ignore_loglevel
> > > video=unifb:1024x600-16@75 root=/dev/nfs rw
> > > nfsroot=/home/nfs/uc3,rsize=1024,wsize=1024
> > > ip=192.168.10.83:192.168.10.82:192.168.10.1:255.255.255.0::eth0:off"
> > >
> > > You should never need to pass the memory size in the command line.
> > > Please ensure that the boot loader always passes the memory size
> > > using your boot protocol (ideally in a device tree).
> > >
> > [Guan] CMDLINE will only be used when no bootloader params.
> > And u-boot params have another cmdline to replace this one.
>
> Well, my point was more specifically that having to provide options on the
> command line that are hardware specific is wrong, independent of whether
> they come from the hardcoded command line or from u-boot.
>
> The command line can of course be used for boot device configuration like
> the NFS root stuff (which I still would not put in the defconfig), but
memory
> size and screen resolution are something that should always be detectable
> these days.
Ok, I will consider to remove memory size and screen resolution lator.
> > > The files look mostly identical. If there is no fundamental
> > > difference between them, I would recommend providing only a single
> > > defconfig file that works on all the machines.
> > [Guan] nb0916 is for netbook and desktop, and CONFIG_MODULES must
> be
> > enabled.
> > Smw0919 is for safety embedded devices, only necessary
> > devices/drivers are integrated.
>
> The defconfig is not typically used without modifications on real systems,
but
> serves as a starting point. If it's reasonable to build a configuration
with all the
> smw0919 device drivers built-in and the additional nb0916 drivers as
loadable
> modules, I guess that would make a reasonable defconfig.
> On smw0919 hardware, you can then just choose not to install the modules
> into the root file system.
Ok.
>
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/dma.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/dma.h 1970-01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/dma.h 2010-11-11
> > > 09:55:29.517572760 +0800
> > > > @@ -0,0 +1,34 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/dma.h
> > >
> > > Just use the asm-generic file.
> > [Guan] when I set MAX_DMA_ADDRESS to PAGE_OFFSET, some pci drivers
> > have problem.
> > I will check it later.
>
> Interesting. You already allow to override MAX_DMA_ADDRESS to
> (PAGE_OFFSET + SZ_128M) when PCI is enabled. It would be good to find out
> (and document!) the signficance of the extra 128MB here.
>
PCI controller in PKUnity-3 masks highest 5-bit for upstream channel, so we
must
Limit the DMA allocation with 128M physical memory for supporting PCI
devices.
> If it turns out to be required, please make a patch to the
asm-generic/dma.h
> file to allow overriding MAX_DMA_ADDRESS in the same way you do in your
> version.
I need to check it later.
I suppose MAX_DMA_ADDRESS is virtual address, so it should be larger than
PAGE_OFFSET.
>
> > > > + * Nothing too fancy for now.
> > > > + *
> > > > + * On UniCore we already have well known fixed virtual addresses
> > > > + imposed by
> > > > + * the architecture such as the vector page which is located at
> > > > + 0xffff0000,
> > >
> > > The comment is copied from ARM, but is it really true on unicore?
> > >
> > [Guan] Yes.
>
> In what way does the architecture enforce this? What are the contents of
> this page? Can you make it an actual VDSO rather than a magic page that
sits
> in the user address space?
Page table is created for vector page and exceptions entry stub.
However, vector page is not in the user address space.
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/highmem.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/highmem.h
1970-
> 01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/highmem.h 2010-
> 11-08
> > > 15:53:09.077836027 +0800
> > > > @@ -0,0 +1,53 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/highmem.h
> > >
> > > What is the maximum amount of memory you support on the 32 bit
> machines?
> > > We're removing all the optimizations for highmem from the kernel
> > > now, so I would recommend not to support this in new architectures
> > > if you can avoid it.
> > [Guan] we need to support 2GB memory.
>
> Would it be an option to have a fixed 2GB/2GB split between user and
> physical address space? That would limit the user addressable range to
> something just under 2GB (because of vmalloc/ioremap pages), but would
> significantly simplify and speed up user memory access from kernel space.
Thanks. I consider not to support highmem in 32-bit address space.
> > > > +/*
> > > > + * Use this value to indicate lack of interrupt
> > > > + * capability
> > > > + */
> > > > +#ifndef NO_IRQ
> > > > +#define NO_IRQ ((unsigned int)(-1))
> > > > +#endif
> > >
> > > This should not be required at all any more.
> > >
> > [Guan] amba bus driver need NO_IRQ
>
> That is a bug, as far as I can tell. I think it should be changed to a
hardcoded (-
> 1) in the amba drivers as we do elsewhere.
>
> Until now, I was not even aware that we had an amba bus driver, and I'm
not
> conviced that it's a good idea to use it either in its current form.
>
> Most of the users of the amba bus currently declare static devices in the
> architecture tree, which is something that breaks a number of assumptions
in
> the device model about the life time of device structures.
Yes. Amba bus and NO_IRQ are removed.
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/pci.h
1970-
> 01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/pci.h
2010-
> 11-08
> > > 17:46:49.475569661 +0800
> > > > @@ -0,0 +1,65 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/mach/pci.h
> > > > +
> > > > +struct hw_pci {
> > > > +#ifdef CONFIG_PCI_DOMAINS
> > > > + int domain;
> > > > +#endif
> > > > + struct list_head buses;
> > > > + int nr_controllers;
> > > > + int (*setup)(int nr, struct pci_sys_data *);
> > > > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *);
> > > > + void (*preinit)(void);
> > > > + void (*postinit)(void);
> > > > + u8 (*swizzle)(struct pci_dev *dev, u8 *pin);
> > > > + int (*map_irq)(struct pci_dev *dev, u8 slot, u8
pin);
> > > > +};
> > > > +
> > > > +/*
> > > > + * Per-controller structure
> > > > + */
> > > > +struct pci_sys_data {
> > > > +#ifdef CONFIG_PCI_DOMAINS
> > > > + int domain;
> > > > +#endif
> > > > + struct list_head node;
> > > > + int busnr; /* primary bus number
*/
> > > > + u64 mem_offset; /* bus->cpu memory mapping
offset
> */
> > > > + unsigned long io_offset; /* bus->cpu IO mapping
offset */
> > > > + struct pci_bus *bus; /* PCI bus
*/
> > > > + struct resource *resource[3]; /* Primary PCI bus resources
*/
> > > > + /* Bridge swizzling
*/
> > > > + u8 (*swizzle)(struct pci_dev *, u8 *);
> > > > + /* IRQ mapping
> */
> > > > + int (*map_irq)(struct pci_dev *, u8, u8);
> > > > + struct hw_pci *hw;
> > > > + void *private_data; /* platform controller
private data
> > */
> > > > +};
> > >
> > > These are two separate abstractions for multiple PCI controllers,
> > > but your code does not even contain one implementation.
> > >
> > > I can only assume that you actually have a PCI implementation, but
> > > if you do not have more than one, you are better off implementing
> > > just the one you have instead of extra wrappers.
> > [Guan] we do have pci bus and pci driver. I have to implement these
> > interface for pci compatibility.
>
> Ok. I would put the actual PCI implementation into the architecture patch
> series then, but get rid of the indirect function pointers.
>
> Do you support multiple PCI domains (root bridges)?
No.
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/mach/time.h
1970-
> 01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/mach/time.h
2010-
> 11-08
> > > 17:48:06.111456784 +0800
> > > > @@ -0,0 +1,52 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/mach/time.h
> > >
> > > Can be removed when you get rid of the machine_desc abstraction.
> > [Guan] sys_timer can't be removed, which is used for device register.
>
> I don't understand. I can only see one sys_timer instance, the puv3_timer,
so
> you can just as well hardcode access to that.
Removed.
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/memblock.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/memblock.h
1970-
> 01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/memblock.h
2010-
> 11-10
> > > 18:59:14.326006751 +0800
> > > > @@ -0,0 +1,22 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/memblock.h
> > >
> > > This too.
> > [Guan] we use memblock as infrastructure, and perhaps it's not good.
>
> No, there is nothing wrong with memblock.
>
> My point was that the indirect function calls through machine_desc that
you
> use here are not needed.
>
> I would also recommend using only memblock and not also bootmem, by
> unconditionally setting CONFIG_NO_BOOTMEM.
I will consider it later.
>
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/swab.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/swab.h
> > > > +#ifndef __UNICORE_SWAB_H__
> > > > +#define __UNICORE_SWAB_H__
> > > > +
> > > > +#include <linux/compiler.h>
> > > > +#include <linux/types.h>
> > > > +
> > > > +#if !defined(__STRICT_ANSI__) || defined(__KERNEL__) # define
> > > > +__SWAB_64_THRU_32__ #endif
> > > > +
> > > > +static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
> > >
> > > Can't you use the __builtin_bswap32/__builtin_bswap64 provided by gcc?
> > [Guan] __bswapsi2 is in libgcc, which is needed by __builtin_bswap32.
>
> You could link against libgcc, see arch/tile/Makefile for an example how
to do
> that.
Yes, it works. I think it's not a good idea. Kernel should be independent on
libgcc.
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/uaccess.h
1970-
> 01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/uaccess.h 2010-
> 11-09
> > > 11:33:30.209566466 +0800
> > > > @@ -0,0 +1,408 @@
> > > > +/*
> > > > + * linux/arch/unicore32/include/asm/uaccess.h
> > >
> > > Please have another look at the asm-generic version of this file.
> > > With the current version it may be useful to base on that.
> > [Guan] __copy_from_user and __copy_to_user is too expensive for 1/2/4
> words.
>
> The asm-generic version of __copy_from_user/__copy_to_user uses
> __builtin_constant_p() to optimize away all this overhead at build time,
so
> even a copy_to_user(dst, src, 4) gets turned into a very small number of
> instructions.
>
> This should work for both cases: when called with a constant size and when
> called from get_user/put_user.
>
> I suppose you can do the same thing in your code.
Ok, I will do this work later.
>
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h
> > > linux-2.6.37.y/arch/unicore32/include/asm/unistd.h
> > > > --- linux-2.6.37-rc1/arch/unicore32/include/asm/unistd.h
1970-01-01
> > > 08:00:00.000000000 +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/include/asm/unistd.h 2010-11-11
> > > 10:47:36.743710466 +0800
> > >
> > > Please use the cleaned-up asm-generic version as arch/tile does now.
> > > You add a lot of overhead otherwise.
> > [Guan] we use __NR_* as the immediate number in soft-interrupt
> instruction.
> > And if the syscall number changed, all software and toolchain
> > need re-compiled.
>
> Yes, I realize that. I think it would be very hard to integrate your
architecture
> code without breaking compatibility, but breaking it once in order to fix
all
> the problems arising from your backwards-compatibility code will make your
> life much easier in the long run.
>
> One possible way to do the migration would be to do the upstream
> integration using only the new ABI, but keep a private patch that reverts
to
> the existing ABI. At some point in the future, you can then declare the
old
> ABI obsolete and drop the patch, supporting only the new ABI.
Good. We will just do the new ABI work recently, and it's a great idea.
>
> > > > diff -uprN -X linux-2.6.37-rc1/Documentation/dontdiff
> > > linux-2.6.37-rc1/arch/unicore32/Kconfig
> > linux-2.6.37.y/arch/unicore32/Kconfig
> > > > --- linux-2.6.37-rc1/arch/unicore32/Kconfig 1970-01-01
> > 08:00:00.000000000
> > > +0800
> > > > +++ linux-2.6.37.y/arch/unicore32/Kconfig 2010-11-11
> > 10:40:02.001225028
> > > +0800
> > > > @@ -0,0 +1,337 @@
> > > > + config ARCH_PUV3
> > > > + bool "PKUnity v3 (SuperK)"
> > > > + select CPU_UCV2
> > > > + select PKUNITY_AMBA
> > > > + select ZONE_DMA
> > > > + select EMBEDDED
> > > > + select GENERIC_CLOCKEVENTS
> > > > + select HAVE_CLK
> > > > + select ARCH_USES_GETTIMEOFFSET
> > > > + select ARCH_REQUIRE_GPIOLIB
> > > > + select ARCH_HAS_CPUFREQ
> > >
> > > You might not want to select ZONE_DMA and
> ARCH_USES_GETTIMEOFFSET,
> > > since you are generally better off without them.
> > [Guan] We need ZONE_DMA for only 128M low memory could be used for
> DMA.
>
> Ok. Is this only for some device drivers, or is this a general limitation?
The ZONE_DMA only limits pci devices, not a general limitation.
However, many chips could be connected to different bus,
so this limit influences global in our architecture.
Guan Xuetao
On Saturday 27 November 2010, Guan Xuetao wrote:
> > > [Guan] when I set MAX_DMA_ADDRESS to PAGE_OFFSET, some pci drivers
> > > have problem.
> > > I will check it later.
> >
> > Interesting. You already allow to override MAX_DMA_ADDRESS to
> > (PAGE_OFFSET + SZ_128M) when PCI is enabled. It would be good to find out
> > (and document!) the signficance of the extra 128MB here.
> >
> PCI controller in PKUnity-3 masks highest 5-bit for upstream channel, so we
> must Limit the DMA allocation with 128M physical memory for supporting PCI
> devices.
>
> > If it turns out to be required, please make a patch to the asm-generic/dma.h
> > file to allow overriding MAX_DMA_ADDRESS in the same way you do in your
> > version.
> I need to check it later.
> I suppose MAX_DMA_ADDRESS is virtual address, so it should be larger than
> PAGE_OFFSET.
I had a closer look again at this. MAX_DMA_ADDRESS is meant to reflect
the size of ZONE_DMA, which it does in your patch.
However, something else is really wrong, in that ZONE_DMA (i.e. GFP_DMA)
should only be used for legacy ISA devices like floppy or parallel port,
not for PCI devices.
If your PCI bus can only do DMA to a limited memory range, you can not
in general use MAX_DMA_ADDRESS/ZONE_DMA for this, but instead need to
use the swiotlb code or a hardware IOMMU in place of your asm/dma-mapping.h
file.
> > > > > + * Nothing too fancy for now.
> > > > > + *
> > > > > + * On UniCore we already have well known fixed virtual addresses
> > > > > + imposed by
> > > > > + * the architecture such as the vector page which is located at
> > > > > + 0xffff0000,
> > > >
> > > > The comment is copied from ARM, but is it really true on unicore?
> > > >
> > > [Guan] Yes.
> >
> > In what way does the architecture enforce this? What are the contents of
> > this page? Can you make it an actual VDSO rather than a magic page that
> > sits in the user address space?
> Page table is created for vector page and exceptions entry stub.
> However, vector page is not in the user address space.
Interesting. So if it's not mapped into user space, why do you even
need to have the vectors at a specific page? I think ARM only maps it
to the high page because that page is shared to user space, while most
architectures just have their interrupt vectors in the linar mapping,
since the hardware typically uses the physical address to find it.
> > I would also recommend using only memblock and not also bootmem, by
> > unconditionally setting CONFIG_NO_BOOTMEM.
> I will consider it later.
ok.
> > You could link against libgcc, see arch/tile/Makefile for an example how
> to do
> > that.
> Yes, it works. I think it's not a good idea. Kernel should be independent on
> libgcc.
I've seen both libgcc based and standalone library approaches in
architectures, but I could never see a strong reaons for one way
or another. Why do you think it should be independent? Licensing
technical problems?
> > > > You might not want to select ZONE_DMA and
> > ARCH_USES_GETTIMEOFFSET,
> > > > since you are generally better off without them.
> > > [Guan] We need ZONE_DMA for only 128M low memory could be used for
> > DMA.
> >
> > Ok. Is this only for some device drivers, or is this a general limitation?
> The ZONE_DMA only limits pci devices, not a general limitation.
> However, many chips could be connected to different bus,
> so this limit influences global in our architecture.
As mentioned above, the drivers behind the PCI (except PCI-ISA bridges) should
be using dma_map_single/dma_alloc_coherent instead of GFP_DMA to get DMA
capable memory, so what you need instead is swiotlb.
Arnd
> -----Original Message-----
> From: [email protected] [mailto:linux-arch-
> [email protected]] On Behalf Of Arnd Bergmann
> Sent: Tuesday, November 30, 2010 12:35 AM
> To: Guan Xuetao
> Cc: 'Greg KH'; 'Andrew Morton'; 'Linus Torvalds'; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] a new UniCore32 arch-dependent patch for linux-
> 2.6.37-rc1
>
> On Saturday 27 November 2010, Guan Xuetao wrote:
>
> > > > [Guan] when I set MAX_DMA_ADDRESS to PAGE_OFFSET, some pci
> drivers
> > > > have problem.
> > > > I will check it later.
> > >
> > > Interesting. You already allow to override MAX_DMA_ADDRESS to
> > > (PAGE_OFFSET + SZ_128M) when PCI is enabled. It would be good to
> > > find out (and document!) the signficance of the extra 128MB here.
> > >
> > PCI controller in PKUnity-3 masks highest 5-bit for upstream channel,
> > so we must Limit the DMA allocation with 128M physical memory for
> > supporting PCI devices.
> >
> > > If it turns out to be required, please make a patch to the
> > > asm-generic/dma.h file to allow overriding MAX_DMA_ADDRESS in the
> > > same way you do in your version.
> > I need to check it later.
> > I suppose MAX_DMA_ADDRESS is virtual address, so it should be larger
> > than PAGE_OFFSET.
>
> I had a closer look again at this. MAX_DMA_ADDRESS is meant to reflect the
> size of ZONE_DMA, which it does in your patch.
>
> However, something else is really wrong, in that ZONE_DMA (i.e. GFP_DMA)
> should only be used for legacy ISA devices like floppy or parallel port,
not for
> PCI devices.
>
> If your PCI bus can only do DMA to a limited memory range, you can not in
> general use MAX_DMA_ADDRESS/ZONE_DMA for this, but instead need to
> use the swiotlb code or a hardware IOMMU in place of your asm/dma-
> mapping.h file.
Ok. Since we have no pci peripherals in our products, I removed these codes.
>
> > > > > > + * Nothing too fancy for now.
> > > > > > + *
> > > > > > + * On UniCore we already have well known fixed virtual
> > > > > > + addresses imposed by
> > > > > > + * the architecture such as the vector page which is located
> > > > > > + at 0xffff0000,
> > > > >
> > > > > The comment is copied from ARM, but is it really true on unicore?
> > > > >
> > > > [Guan] Yes.
> > >
> > > In what way does the architecture enforce this? What are the
> > > contents of this page? Can you make it an actual VDSO rather than a
> > > magic page that sits in the user address space?
> > Page table is created for vector page and exceptions entry stub.
> > However, vector page is not in the user address space.
>
> Interesting. So if it's not mapped into user space, why do you even need
to
> have the vectors at a specific page? I think ARM only maps it to the high
page
> because that page is shared to user space, while most architectures just
have
> their interrupt vectors in the linar mapping, since the hardware typically
uses
> the physical address to find it.
In UniCore-32, when MMU enabled, vector page address is virtual address.
> > > You could link against libgcc, see arch/tile/Makefile for an example
> > > how
> > to do
> > > that.
> > Yes, it works. I think it's not a good idea. Kernel should be
> > independent on libgcc.
>
> I've seen both libgcc based and standalone library approaches in
> architectures, but I could never see a strong reaons for one way or
another.
> Why do you think it should be independent? Licensing technical problems?
No license problem, just my understanding. Thanks.
>
> > > > > You might not want to select ZONE_DMA and
> > > ARCH_USES_GETTIMEOFFSET,
> > > > > since you are generally better off without them.
> > > > [Guan] We need ZONE_DMA for only 128M low memory could be used
> for
> > > DMA.
> > >
> > > Ok. Is this only for some device drivers, or is this a general
limitation?
> > The ZONE_DMA only limits pci devices, not a general limitation.
> > However, many chips could be connected to different bus, so this limit
> > influences global in our architecture.
>
> As mentioned above, the drivers behind the PCI (except PCI-ISA bridges)
> should be using dma_map_single/dma_alloc_coherent instead of GFP_DMA
> to get DMA capable memory, so what you need instead is swiotlb.
Ok, thanks.
Guan Xuetao
On Tuesday 30 November 2010, Guan Xuetao wrote:
> > On Saturday 27 November 2010, Guan Xuetao wrote:
> > > >
> > > > In what way does the architecture enforce this? What are the
> > > > contents of this page? Can you make it an actual VDSO rather than a
> > > > magic page that sits in the user address space?
> > > Page table is created for vector page and exceptions entry stub.
> > > However, vector page is not in the user address space.
> >
> > Interesting. So if it's not mapped into user space, why do you even need to
> > have the vectors at a specific page? I think ARM only maps it to the high page
> > because that page is shared to user space, while most architectures just have
> > their interrupt vectors in the linar mapping, since the hardware typically uses
> > the physical address to find it.
>
> In UniCore-32, when MMU enabled, vector page address is virtual address.
Ok. I see.
The arch_exit_mmap() and arch_setup_additional_pages() functions seems to refer to
how ARM maps this page into user space as well, you should probably change that.
I've finished the second half of the code review now, I'll follow up with an
email but drop the personal Cc to everyone besides us, in case people are getting
bored from the discussion by now ;-)
Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Tuesday, November 30, 2010 11:27 PM
> To: Guan Xuetao
> Cc: 'Greg KH'; 'Andrew Morton'; 'Linus Torvalds'; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] a new UniCore32 arch-dependent patch for linux-
> 2.6.37-rc1
>
> On Tuesday 30 November 2010, Guan Xuetao wrote:
> > > On Saturday 27 November 2010, Guan Xuetao wrote:
> > > > >
> > > > > In what way does the architecture enforce this? What are the
> > > > > contents of this page? Can you make it an actual VDSO rather
> > > > > than a magic page that sits in the user address space?
> > > > Page table is created for vector page and exceptions entry stub.
> > > > However, vector page is not in the user address space.
> > >
> > > Interesting. So if it's not mapped into user space, why do you even
> > > need to have the vectors at a specific page? I think ARM only maps
> > > it to the high page because that page is shared to user space, while
> > > most architectures just have their interrupt vectors in the linar
> > > mapping, since the hardware typically uses the physical address to
find it.
> >
> > In UniCore-32, when MMU enabled, vector page address is virtual address.
>
> Ok. I see.
>
> The arch_exit_mmap() and arch_setup_additional_pages() functions seems
> to refer to how ARM maps this page into user space as well, you should
> probably change that.
Vector page is also used as kuser page, so arch_setup_additional_pages need
to
Map it into user space.
>
> I've finished the second half of the code review now, I'll follow up with
an
> email but drop the personal Cc to everyone besides us, in case people are
> getting bored from the discussion by now ;-)
Thanks again.
Guan Xuetao