Embedded Linux development
 help / color / mirror / Atom feed
* Re: Incompatible CFLAGS for kernel module
From: Shaz @ 2008-07-05  5:33 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: linux-embedded, ray.sunjae.lee, tpm-emulator-user,
	tpm-emulator-devel, Security Engineering Archive
In-Reply-To: <20080704203255.GB19110@uranus.ravnborg.org>

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

On Sat, Jul 5, 2008 at 1:32 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Fri, Jul 04, 2008 at 10:34:56PM +0500, Shaz wrote:
>> Hi,
>>
>> I am porting tpm emulator to versatilepb, which is an arm platform. I
>> have some problem with CFLAGS from /arch/arm/Makefile when building
>> the kernel module for this software. I cannot figure out what to do in
>> this case. Following is the error:
>>
>> [root@develbox tpm_emulator-0.5]# make
>> Making all in tpmd
>> make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd'
>> make[1]: Nothing to be done for `all'.
>> make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd'
>> Making all in tpmd_dev
>> make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd_dev'
>>   CC [M]  /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o
>> cc1: error: unrecognized command line option "-mlittle-endian"
>> cc1: error: unrecognized command line option "-mapcs"
>> cc1: error: unrecognized command line option "-mno-sched-prolog"
>> cc1: error: unrecognized command line option "-mabi=aapcs-linux"
>> cc1: error: unrecognized command line option "-mno-thumb-interwork"
>> /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
>> (armv4t) for -march= switch
>> /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
>> (arm9tdmi) for -mtune= switch
>> make[3]: *** [/embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o] Error 1
>> make[2]: *** [_module_/embedded/tpm_emulator-0.5/tpmd_dev] Error 2
>> make[1]: *** [all] Error 2
>> make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd_dev'
>> make: *** [all-recursive] Error 255
>>
>> kindly help me understand the core issue here and point out some solutions.
>
> A quick guess is that you use your host gcc and not your target gcc.
> You most likely have to set CROSS_COMPILE=...
>
> Try with "make V=1" to see the exact gcc command line.
>
> If you continue to rn into trouble please post your Makefile.
I tried make V=1 but the loads of info was given for the things that
went right but nothing noticeable where the error is.
I've attached my arch/arm/Makefile. My build, host and target are on
same system. i am emulating the board with Qemu. The process is very
conventional. I've mounted my rootfs with -o loop and kernel and extra
modules go there with INSTALL_MOD_PATH and INSTALL_MOD_DIR. ARCH=arm
and CROSS_COMPILE=arm-linux-
>
>        Sam
>



-- 
Shaz

[-- Attachment #2: Makefile --]
[-- Type: application/octet-stream, Size: 9736 bytes --]

#
# arch/arm/Makefile
#
# This file is included by the global makefile so that you can add your own
# architecture-specific flags and dependencies.
#
# This file is subject to the terms and conditions of the GNU General Public
# License.  See the file "COPYING" in the main directory of this archive
# for more details.
#
# Copyright (C) 1995-2001 by Russell King

LDFLAGS_vmlinux	:=-p --no-undefined -X
CPPFLAGS_vmlinux.lds = -DTEXT_OFFSET=$(TEXT_OFFSET)
OBJCOPYFLAGS	:=-O binary -R .note -R .note.gnu.build-id -R .comment -S
GZFLAGS		:=-9
#KBUILD_CFLAGS	+=-pipe
# Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
KBUILD_CFLAGS	+=$(call cc-option,-marm,)

# Do not use arch/arm/defconfig - it's always outdated.
# Select a platform tht is kept up-to-date
KBUILD_DEFCONFIG := versatile_defconfig

# defines filename extension depending memory manement type.
ifeq ($(CONFIG_MMU),)
MMUEXT		:= -nommu
endif

ifeq ($(CONFIG_FRAME_POINTER),y)
KBUILD_CFLAGS	+=-fno-omit-frame-pointer -mapcs -mno-sched-prolog
endif

ifeq ($(CONFIG_CPU_BIG_ENDIAN),y)
KBUILD_CPPFLAGS	+= -mbig-endian
AS		+= -EB
LD		+= -EB
else
KBUILD_CPPFLAGS	+= -mlittle-endian
AS		+= -EL
LD		+= -EL
endif

comma = ,

# This selects which instruction set is used.
# Note that GCC does not numerically define an architecture version
# macro, but instead defines a whole series of macros which makes
# testing for a specific architecture or later rather impossible.
arch-$(CONFIG_CPU_32v7)		:=-D__LINUX_ARM_ARCH__=7 $(call cc-option,-march=armv7a,-march=armv5t -Wa$(comma)-march=armv7a)
arch-$(CONFIG_CPU_32v6)		:=-D__LINUX_ARM_ARCH__=6 $(call cc-option,-march=armv6,-march=armv5t -Wa$(comma)-march=armv6)
# Only override the compiler option if ARMv6. The ARMv6K extensions are
# always available in ARMv7
ifeq ($(CONFIG_CPU_32v6),y)
arch-$(CONFIG_CPU_32v6K)	:=-D__LINUX_ARM_ARCH__=6 $(call cc-option,-march=armv6k,-march=armv5t -Wa$(comma)-march=armv6k)
endif
arch-$(CONFIG_CPU_32v5)		:=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
arch-$(CONFIG_CPU_32v4T)	:=-D__LINUX_ARM_ARCH__=4 -march=armv4t
arch-$(CONFIG_CPU_32v4)		:=-D__LINUX_ARM_ARCH__=4 -march=armv4
arch-$(CONFIG_CPU_32v3)		:=-D__LINUX_ARM_ARCH__=3 -march=armv3

# This selects how we optimise for the processor.
tune-$(CONFIG_CPU_ARM610)	:=-mtune=arm610
tune-$(CONFIG_CPU_ARM710)	:=-mtune=arm710
tune-$(CONFIG_CPU_ARM7TDMI)	:=-mtune=arm7tdmi
tune-$(CONFIG_CPU_ARM720T)	:=-mtune=arm7tdmi
tune-$(CONFIG_CPU_ARM740T)	:=-mtune=arm7tdmi
tune-$(CONFIG_CPU_ARM9TDMI)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_ARM940T)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_ARM946T)	:=$(call cc-option,-mtune=arm9e,-mtune=arm9tdmi)
tune-$(CONFIG_CPU_ARM920T)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_ARM922T)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_ARM925T)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_ARM926T)	:=-mtune=arm9tdmi
tune-$(CONFIG_CPU_SA110)	:=-mtune=strongarm110
tune-$(CONFIG_CPU_SA1100)	:=-mtune=strongarm1100
tune-$(CONFIG_CPU_XSCALE)	:=$(call cc-option,-mtune=xscale,-mtune=strongarm110) -Wa,-mcpu=xscale
tune-$(CONFIG_CPU_XSC3)		:=$(call cc-option,-mtune=xscale,-mtune=strongarm110) -Wa,-mcpu=xscale
tune-$(CONFIG_CPU_V6)		:=$(call cc-option,-mtune=arm1136j-s,-mtune=strongarm)

ifeq ($(CONFIG_AEABI),y)
CFLAGS_ABI	:=-mabi=aapcs-linux -mno-thumb-interwork
else
CFLAGS_ABI	:=$(call cc-option,-mapcs-32,-mabi=apcs-gnu) $(call cc-option,-mno-thumb-interwork,)
endif

# Need -Uarm for gcc < 3.x
KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(arch-y) $(tune-y) -msoft-float

CHECKFLAGS	+= -D__arm__

#Default value
head-y		:= arch/arm/kernel/head$(MMUEXT).o arch/arm/kernel/init_task.o
textofs-y	:= 0x00008000

 machine-$(CONFIG_ARCH_RPC)	   := rpc
 machine-$(CONFIG_ARCH_EBSA110)	   := ebsa110
 machine-$(CONFIG_ARCH_CLPS7500)   := clps7500
  incdir-$(CONFIG_ARCH_CLPS7500)   := cl7500
 machine-$(CONFIG_FOOTBRIDGE)	   := footbridge
  incdir-$(CONFIG_FOOTBRIDGE)	   := ebsa285
 machine-$(CONFIG_ARCH_CO285)	   := footbridge
  incdir-$(CONFIG_ARCH_CO285)	   := ebsa285
 machine-$(CONFIG_ARCH_SHARK)	   := shark
 machine-$(CONFIG_ARCH_SA1100)	   := sa1100
ifeq ($(CONFIG_ARCH_SA1100),y)
# SA1111 DMA bug: we don't want the kernel to live in precious DMA-able memory
 textofs-$(CONFIG_SA1111)	   := 0x00208000
endif
 machine-$(CONFIG_ARCH_PXA)	   := pxa
 machine-$(CONFIG_ARCH_L7200)	   := l7200
 machine-$(CONFIG_ARCH_INTEGRATOR) := integrator
 textofs-$(CONFIG_ARCH_CLPS711X)   := 0x00028000
 machine-$(CONFIG_ARCH_CLPS711X)   := clps711x
 machine-$(CONFIG_ARCH_IOP32X)	   := iop32x
 machine-$(CONFIG_ARCH_IOP33X)	   := iop33x
 machine-$(CONFIG_ARCH_IOP13XX)	   := iop13xx
 machine-$(CONFIG_ARCH_IXP4XX)	   := ixp4xx
 machine-$(CONFIG_ARCH_IXP2000)    := ixp2000
 machine-$(CONFIG_ARCH_IXP23XX)    := ixp23xx
 machine-$(CONFIG_ARCH_OMAP1)	   := omap1
 machine-$(CONFIG_ARCH_OMAP2)	   := omap2
  incdir-$(CONFIG_ARCH_OMAP)	   := omap
 machine-$(CONFIG_ARCH_S3C2410)	   := s3c2410
 machine-$(CONFIG_ARCH_LH7A40X)	   := lh7a40x
 machine-$(CONFIG_ARCH_VERSATILE)  := versatile
 machine-$(CONFIG_ARCH_IMX)	   := imx
 machine-$(CONFIG_ARCH_H720X)	   := h720x
 machine-$(CONFIG_ARCH_AAEC2000)   := aaec2000
 machine-$(CONFIG_ARCH_REALVIEW)   := realview
 machine-$(CONFIG_ARCH_AT91)	   := at91
 machine-$(CONFIG_ARCH_EP93XX)	   := ep93xx
 machine-$(CONFIG_ARCH_PNX4008)	   := pnx4008
 machine-$(CONFIG_ARCH_NETX)	   := netx
 machine-$(CONFIG_ARCH_NS9XXX)	   := ns9xxx
 textofs-$(CONFIG_ARCH_NS9XXX)	   := 0x00108000
 machine-$(CONFIG_ARCH_DAVINCI)	   := davinci
 machine-$(CONFIG_ARCH_KS8695)     := ks8695
  incdir-$(CONFIG_ARCH_MXC)	   := mxc
 machine-$(CONFIG_ARCH_MX3)	   := mx3
 machine-$(CONFIG_ARCH_ORION)	   := orion
 machine-$(CONFIG_ARCH_MSM7X00A)   := msm

ifeq ($(CONFIG_ARCH_EBSA110),y)
# This is what happens if you forget the IOCS16 line.
# PCMCIA cards stop working.
CFLAGS_3c589_cs.o :=-DISA_SIXTEEN_BIT_PERIPHERAL
export CFLAGS_3c589_cs.o
endif

# The byte offset of the kernel image in RAM from the start of RAM.
TEXT_OFFSET := $(textofs-y)

ifeq ($(incdir-y),)
incdir-y := $(machine-y)
endif
INCDIR   := arch-$(incdir-y)

ifneq ($(machine-y),)
MACHINE  := arch/arm/mach-$(machine-y)/
else
MACHINE  :=
endif

export	TEXT_OFFSET GZFLAGS MMUEXT

# Do we have FASTFPE?
FASTFPE		:=arch/arm/fastfpe
ifeq ($(FASTFPE),$(wildcard $(FASTFPE)))
FASTFPE_OBJ	:=$(FASTFPE)/
endif

# If we have a machine-specific directory, then include it in the build.
core-y				+= arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
core-y				+= $(MACHINE)
core-$(CONFIG_ARCH_S3C2410)	+= arch/arm/mach-s3c2400/
core-$(CONFIG_ARCH_S3C2410)	+= arch/arm/mach-s3c2412/
core-$(CONFIG_ARCH_S3C2410)	+= arch/arm/mach-s3c2440/
core-$(CONFIG_ARCH_S3C2410)	+= arch/arm/mach-s3c2442/
core-$(CONFIG_ARCH_S3C2410)	+= arch/arm/mach-s3c2443/
core-$(CONFIG_FPE_NWFPE)	+= arch/arm/nwfpe/
core-$(CONFIG_FPE_FASTFPE)	+= $(FASTFPE_OBJ)
core-$(CONFIG_VFP)		+= arch/arm/vfp/

# If we have a common platform directory, then include it in the build.
core-$(CONFIG_PLAT_IOP)		+= arch/arm/plat-iop/
core-$(CONFIG_ARCH_OMAP)	+= arch/arm/plat-omap/
core-$(CONFIG_PLAT_S3C24XX)		+= arch/arm/plat-s3c24xx/
core-$(CONFIG_ARCH_MXC)		+= arch/arm/plat-mxc/

drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
drivers-$(CONFIG_ARCH_CLPS7500)	+= drivers/acorn/char/
drivers-$(CONFIG_ARCH_L7200)	+= drivers/acorn/char/

libs-y				:= arch/arm/lib/ $(libs-y)

# Default target when executing plain make
ifeq ($(CONFIG_XIP_KERNEL),y)
KBUILD_IMAGE := xipImage
else
KBUILD_IMAGE := zImage
endif

all:	$(KBUILD_IMAGE)

boot := arch/arm/boot

#	Update machine arch and proc symlinks if something which affects
#	them changed.  We use .arch to indicate when they were updated
#	last, otherwise make uses the target directory mtime.

include/asm-arm/.arch: $(wildcard include/config/arch/*.h) include/config/auto.conf
	@echo '  SYMLINK include/asm-arm/arch -> include/asm-arm/$(INCDIR)'
ifneq ($(KBUILD_SRC),)
	$(Q)mkdir -p include/asm-arm
	$(Q)ln -fsn $(srctree)/include/asm-arm/$(INCDIR) include/asm-arm/arch
else
	$(Q)ln -fsn $(INCDIR) include/asm-arm/arch
endif
	@touch $@

archprepare: maketools

PHONY += maketools FORCE
maketools: include/linux/version.h include/asm-arm/.arch FORCE
	$(Q)$(MAKE) $(build)=arch/arm/tools include/asm-arm/mach-types.h

# Convert bzImage to zImage
bzImage: zImage

zImage Image xipImage bootpImage uImage: vmlinux
	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@

zinstall install: vmlinux
	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@

CLEAN_FILES += include/asm-arm/mach-types.h \
	       include/asm-arm/arch include/asm-arm/.arch

# We use MRPROPER_FILES and CLEAN_FILES now
archclean:
	$(Q)$(MAKE) $(clean)=$(boot)

# My testing targets (bypasses dependencies)
bp:;	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/bootpImage
i zi:;	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@


define archhelp
  echo  '* zImage        - Compressed kernel image (arch/$(ARCH)/boot/zImage)'
  echo  '  Image         - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
  echo  '* xipImage      - XIP kernel image, if configured (arch/$(ARCH)/boot/xipImage)'
  echo  '  uImage        - U-Boot wrapped zImage'
  echo  '  bootpImage    - Combined zImage and initial RAM disk' 
  echo  '                  (supply initrd image via make variable INITRD=<path>)'
  echo  '  install       - Install uncompressed kernel'
  echo  '  zinstall      - Install compressed kernel'
  echo  '                  Install using (your) ~/bin/installkernel or'
  echo  '                  (distribution) /sbin/installkernel or'
  echo  '                  install to $$(INSTALL_PATH) and run lilo'
endef

^ permalink raw reply

* Re: Incompatible CFLAGS for kernel module
From: Sam Ravnborg @ 2008-07-04 20:32 UTC (permalink / raw)
  To: Shaz
  Cc: linux-embedded, ray.sunjae.lee, tpm-emulator-user,
	tpm-emulator-devel, Security Engineering Archive
In-Reply-To: <7b740b700807041034m1df5ad54x4b1112d74905dc7c@mail.gmail.com>

On Fri, Jul 04, 2008 at 10:34:56PM +0500, Shaz wrote:
> Hi,
> 
> I am porting tpm emulator to versatilepb, which is an arm platform. I
> have some problem with CFLAGS from /arch/arm/Makefile when building
> the kernel module for this software. I cannot figure out what to do in
> this case. Following is the error:
> 
> [root@develbox tpm_emulator-0.5]# make
> Making all in tpmd
> make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd'
> make[1]: Nothing to be done for `all'.
> make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd'
> Making all in tpmd_dev
> make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd_dev'
>   CC [M]  /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o
> cc1: error: unrecognized command line option "-mlittle-endian"
> cc1: error: unrecognized command line option "-mapcs"
> cc1: error: unrecognized command line option "-mno-sched-prolog"
> cc1: error: unrecognized command line option "-mabi=aapcs-linux"
> cc1: error: unrecognized command line option "-mno-thumb-interwork"
> /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
> (armv4t) for -march= switch
> /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
> (arm9tdmi) for -mtune= switch
> make[3]: *** [/embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o] Error 1
> make[2]: *** [_module_/embedded/tpm_emulator-0.5/tpmd_dev] Error 2
> make[1]: *** [all] Error 2
> make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd_dev'
> make: *** [all-recursive] Error 255
> 
> kindly help me understand the core issue here and point out some solutions.

A quick guess is that you use your host gcc and not your target gcc.
You most likely have to set CROSS_COMPILE=...

Try with "make V=1" to see the exact gcc command line.

If you continue to rn into trouble please post your Makefile.

	Sam

^ permalink raw reply

* Incompatible CFLAGS for kernel module
From: Shaz @ 2008-07-04 17:34 UTC (permalink / raw)
  To: linux-embedded
  Cc: ray.sunjae.lee, tpm-emulator-user, tpm-emulator-devel,
	Security Engineering Archive

Hi,

I am porting tpm emulator to versatilepb, which is an arm platform. I
have some problem with CFLAGS from /arch/arm/Makefile when building
the kernel module for this software. I cannot figure out what to do in
this case. Following is the error:

[root@develbox tpm_emulator-0.5]# make
Making all in tpmd
make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd'
Making all in tpmd_dev
make[1]: Entering directory `/embedded/tpm_emulator-0.5/tpmd_dev'
  CC [M]  /embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o
cc1: error: unrecognized command line option "-mlittle-endian"
cc1: error: unrecognized command line option "-mapcs"
cc1: error: unrecognized command line option "-mno-sched-prolog"
cc1: error: unrecognized command line option "-mabi=aapcs-linux"
cc1: error: unrecognized command line option "-mno-thumb-interwork"
/embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
(armv4t) for -march= switch
/embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.c:1: error: bad value
(arm9tdmi) for -mtune= switch
make[3]: *** [/embedded/tpm_emulator-0.5/tpmd_dev/tpmd_dev.o] Error 1
make[2]: *** [_module_/embedded/tpm_emulator-0.5/tpmd_dev] Error 2
make[1]: *** [all] Error 2
make[1]: Leaving directory `/embedded/tpm_emulator-0.5/tpmd_dev'
make: *** [all-recursive] Error 255

kindly help me understand the core issue here and point out some solutions.

-- 
Shaz

^ permalink raw reply

* Re: [PATCH v4 5/6] dmaengine: Driver for the Synopsys DesignWare DMA controller
From: Haavard Skinnemoen @ 2008-07-04 16:10 UTC (permalink / raw)
  To: Sosnowski, Maciej
  Cc: Williams, Dan J, drzeus-list, lkml, linux-embedded, kernel,
	Nelson, Shannon, david-b
In-Reply-To: <7F38996F7185A24AB9071ED4950AD8C101C4D314@swsmsx413.ger.corp.intel.com>

On Fri, 4 Jul 2008 16:33:53 +0100
"Sosnowski, Maciej" <maciej.sosnowski@intel.com> wrote:
> Coulpe of questions and comments from my side below.
> Apart from that the code looks fine to me.
> 
> Acked-by: Maciej Sosnowski <maciej.sosnowski@intel.com>

Thanks a lot for reviewing!

> > +/* Called with dwc->lock held and bh disabled */
> > +static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc
> *first)
> > +{
> > +       struct dw_dma   *dw = to_dw_dma(dwc->chan.device);
> > +
> > +       /* ASSERT:  channel is idle */
> > +       if (dma_readl(dw, CH_EN) & dwc->mask) {
> > +               dev_err(&dwc->chan.dev,
> > +                       "BUG: Attempted to start non-idle channel\n");
> > +               dev_err(&dwc->chan.dev,
> > +                       "  SAR: 0x%x DAR: 0x%x LLP: 0x%x CTL:
> 0x%x:%08x\n",
> > +                       channel_readl(dwc, SAR),
> > +                       channel_readl(dwc, DAR),
> > +                       channel_readl(dwc, LLP),
> > +                       channel_readl(dwc, CTL_HI),
> > +                       channel_readl(dwc, CTL_LO));
> > +
> > +               /* The tasklet will hopefully advance the queue... */
> > +               return;
> 
> Should not at this point an error status be returned 
> so that it can be handled accordingly by dwc_dostart() caller?

There's not a whole lot of meaningful things to do for the caller. It
should never happen in the first place, but if the channel _is_ active
at this point, we will eventually get an xfer complete interrupt when
the currently pending transfers are done. The descriptors have already
been added to the list, so the driver should recover from this kind of
bug automatically.

I've never actually triggered this code, so I can't really say for
certain that it works, but at least in theory it makes much more sense
to fix things up when the channel eventually becomes idle.

> > +       ctllo = DWC_DEFAULT_CTLLO
> > +                       | DWC_CTLL_DST_WIDTH(dst_width)
> > +                       | DWC_CTLL_SRC_WIDTH(src_width)
> > +                       | DWC_CTLL_DST_INC
> > +                       | DWC_CTLL_SRC_INC
> > +                       | DWC_CTLL_FC_M2M;
> > +       prev = first = NULL;
> > +
> > +       for (offset = 0; offset < len; offset += xfer_count <<
> src_width) {
> > +               xfer_count = min_t(size_t, (len - offset) >>
> src_width,
> > +                               DWC_MAX_COUNT);
> 
> Here it looks like the maximum xfer_count value can change - it depends
> on src_width, 
> so it may be different for different transactions.
> Is that ok?

Yes, the maximum tranfer count is defined as the maximum number of
source transactions on the bus. So if the controller is set up to do 32
bits at a time on the source side, the maximum transfer _length_ is
four times the maximum transfer _count_.

The value written to the descriptor is also a transaction count, not a
byte count.

> This driver does not perform any self-test during initialization.
> What about adding some initial HW checking?

I'm not sure if it makes a lot of sense -- this device is typically
integrated on the same silicon as the CPU, so if there are any issues
with the DMA controller, they should be caught during production
testing.

I'm using the dmatest module for validating the driver, so I feel the
self-test stuff becomes somewhat redundant.

Haavard

^ permalink raw reply

* RE: [PATCH v4 5/6] dmaengine: Driver for the Synopsys DesignWare DMA controller
From: Sosnowski, Maciej @ 2008-07-04 15:33 UTC (permalink / raw)
  To: haavard.skinnemoen
  Cc: Williams, Dan J, drzeus-list, lkml, linux-embedded, kernel,
	Nelson, Shannon, david-b
In-Reply-To: <f12847240806270224h696e78a1v4a1aa6a87fb4a171@mail.gmail.com>

> ---------- Original message ----------
> From: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> Date: Jun 26, 2008 3:23 PM
> Subject: [PATCH v4 5/6] dmaengine: Driver for the Synopsys DesignWare
> DMA controller
> To: Dan Williams <dan.j.williams@intel.com>, Pierre Ossman
> <drzeus-list@drzeus.cx>
> Cc: linux-kernel@vger.kernel.org, linux-embedded@vger.kernel.org,
> kernel@avr32linux.org, shannon.nelson@intel.com, David Brownell
> <david-b@pacbell.net>, Haavard Skinnemoen
> <haavard.skinnemoen@atmel.com>
> 
> 
> This adds a driver for the Synopsys DesignWare DMA controller (aka
> DMACA on AVR32 systems.) This DMA controller can be found integrated
> on the AT32AP7000 chip and is primarily meant for peripheral DMA
> transfer, but can also be used for memory-to-memory transfers.
> 
> This patch is based on a driver from David Brownell which was based on
> an older version of the DMA Engine framework. It also implements the
> proposed extensions to the DMA Engine API for slave DMA operations.
> 
> The dmatest client shows no problems, but there may still be room for
> improvement performance-wise. DMA slave transfer performance is
> definitely "good enough"; reading 100 MiB from an SD card running at
~20
> MHz yields ~7.2 MiB/s average transfer rate.
> 
> Full documentation for this controller can be found in the Synopsys
> DW AHB DMAC Databook:
> 
>
http://www.synopsys.com/designware/docs/iip/DW_ahb_dmac/latest/doc/dw_ah
b_dmac_db.pdf
> 
> The controller has lots of implementation options, so it's usually a
> good idea to check the data sheet of the chip it's intergrated on as
> well. The AT32AP7000 data sheet can be found here:
> 
> http://www.atmel.com/dyn/products/datasheets.asp?family_id=682
> 
> Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> 
> Changes since v3:
>  * Update to latest DMA engine and DMA slave APIs
>  * Embed the hw descriptor into the sw descriptor
>  * Clean up and update MODULE_DESCRIPTION, copyright date, etc.
> 
> Changes since v2:
>  * Dequeue all pending transfers in terminate_all()
>  * Rename dw_dmac.h -> dw_dmac_regs.h
>  * Define and use controller-specific dma_slave data
>  * Fix up a few outdated comments
>  * Define hardware registers as structs (doesn't generate better
>    code, unfortunately, but it looks nicer.)
>  * Get number of channels from platform_data instead of hardcoding it
>    based on CONFIG_WHATEVER_CPU.
>  * Give slave clients exclusive access to the channel

Coulpe of questions and comments from my side below.
Apart from that the code looks fine to me.

Acked-by: Maciej Sosnowski <maciej.sosnowski@intel.com>

> ---
>  arch/avr32/mach-at32ap/at32ap700x.c        |   26 +-
>  drivers/dma/Kconfig                        |    9 +
>  drivers/dma/Makefile                       |    1 +
>  drivers/dma/dw_dmac.c                      | 1105
>  ++++++++++++++++++++++++++++ drivers/dma/dw_dmac_regs.h
| 
>  224 ++++++ include/asm-avr32/arch-at32ap/at32ap700x.h |   16 +
>  include/linux/dw_dmac.h                    |   62 ++
>  7 files changed, 1430 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/dma/dw_dmac.c
>  create mode 100644 drivers/dma/dw_dmac_regs.h
>  create mode 100644 include/linux/dw_dmac.h
> 
> diff --git a/arch/avr32/mach-at32ap/at32ap700x.c
> b/arch/avr32/mach-at32ap/at32ap700x.c
> index 0f24b4f..2b92047 100644
> --- a/arch/avr32/mach-at32ap/at32ap700x.c
> +++ b/arch/avr32/mach-at32ap/at32ap700x.c
> @@ -599,6 +599,17 @@ static void __init genclk_init_parent(struct clk
*clk)
>        clk->parent = parent;
>  }
> 
> +static struct dw_dma_platform_data dw_dmac0_data = {
> +       .nr_channels    = 3,
> +};
> +
> +static struct resource dw_dmac0_resource[] = {
> +       PBMEM(0xff200000),
> +       IRQ(2),
> +};
> +DEFINE_DEV_DATA(dw_dmac, 0);
> +DEV_CLK(hclk, dw_dmac0, hsb, 10);
> +
>  /*
--------------------------------------------------------------------
>  *  System peripherals
>  *
-------------------------------------------------------------------- */
> @@ -705,17 +716,6 @@ static struct clk pico_clk = {
>        .users          = 1,
>  };
> 
> -static struct resource dmaca0_resource[] = {
> -       {
> -               .start  = 0xff200000,
> -               .end    = 0xff20ffff,
> -               .flags  = IORESOURCE_MEM,
> -       },
> -       IRQ(2),
> -};
> -DEFINE_DEV(dmaca, 0);
> -DEV_CLK(hclk, dmaca0, hsb, 10);
> -
>  /*
--------------------------------------------------------------------
>  * HMATRIX
>  *
-------------------------------------------------------------------- */
> @@ -828,7 +828,7 @@ void __init at32_add_system_devices(void)
>        platform_device_register(&at32_eic0_device);
>        platform_device_register(&smc0_device);
>        platform_device_register(&pdc_device);
> -       platform_device_register(&dmaca0_device);
> +       platform_device_register(&dw_dmac0_device);
> 
>        platform_device_register(&at32_tcb0_device);
>        platform_device_register(&at32_tcb1_device);
> @@ -1891,7 +1891,7 @@ struct clk *at32_clock_list[] = {
>        &smc0_mck,
>        &pdc_hclk,
>        &pdc_pclk,
> -       &dmaca0_hclk,
> +       &dw_dmac0_hclk,
>        &pico_clk,
>        &pio0_mck,
>        &pio1_mck,
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 2ac09be..4fac4e3 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -37,6 +37,15 @@ config INTEL_IOP_ADMA
>        help
>          Enable support for the Intel(R) IOP Series RAID engines.
> 
> +config DW_DMAC
> +       tristate "Synopsys DesignWare AHB DMA support"
> +       depends on AVR32
> +       select DMA_ENGINE
> +       default y if CPU_AT32AP7000
> +       help
> +         Support the Synopsys DesignWare AHB DMA controller.  This
> +         can be integrated in chips such as the Atmel AT32ap7000.
> +
>  config FSL_DMA
>        bool "Freescale MPC85xx/MPC83xx DMA support"
>        depends on PPC
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 2ff6d7f..beebae4 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
>  obj-$(CONFIG_NET_DMA) += iovlock.o
>  obj-$(CONFIG_INTEL_IOATDMA) += ioatdma.o
> +obj-$(CONFIG_DW_DMAC) += dw_dmac.o
>  ioatdma-objs := ioat.o ioat_dma.o ioat_dca.o
>  obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
>  obj-$(CONFIG_FSL_DMA) += fsldma.o
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> new file mode 100644
> index 0000000..e5389e1
> --- /dev/null
> +++ b/drivers/dma/dw_dmac.c
> @@ -0,0 +1,1105 @@
> +/*
> + * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
> + * AVR32 systems.)
> + *
> + * Copyright (C) 2007-2008 Atmel Corporation
> + *
> + * 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.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "dw_dmac_regs.h"
> +
> +/*
> + * This supports the Synopsys "DesignWare AHB Central DMA
Controller",
> + * (DW_ahb_dmac) which is used with various AMBA 2.0 systems (not all
> + * of which use ARM any more).  See the "Databook" from Synopsys for
> + * information beyond what licensees probably provide.
> + *
> + * The driver has currently been tested only with the Atmel
AT32AP7000,
> + * which does not support descriptor writeback.
> + */
> +
> +/* NOTE:  DMS+SMS is system-specific. We should get this information
> + * from the platform code somehow.
> + */
> +#define DWC_DEFAULT_CTLLO      (DWC_CTLL_DST_MSIZE(0)          \
> +                               | DWC_CTLL_SRC_MSIZE(0)         \
> +                               | DWC_CTLL_DMS(0)               \
> +                               | DWC_CTLL_SMS(1)               \
> +                               | DWC_CTLL_LLP_D_EN             \
> +                               | DWC_CTLL_LLP_S_EN)
> +
> +/*
> + * This is configuration-dependent and usually a funny size like
4095.
> + * Let's round it down to the nearest power of two.
> + *
> + * Note that this is a transfer count, i.e. if we transfer 32-bit
> + * words, we can do 8192 bytes per descriptor.
> + *
> + * This parameter is also system-specific.
> + */
> +#define DWC_MAX_COUNT  2048U
> +
> +/*
> + * Number of descriptors to allocate for each channel. This should be
> + * made configurable somehow; preferably, the clients (at least the
> + * ones using slave transfers) should be able to give us a hint.
> + */
> +#define NR_DESCS_PER_CHANNEL   64
> +
>
+/*---------------------------------------------------------------------
-*/
> +
> +/*
> + * Because we're not relying on writeback from the controller (it may
not
> + * even be configured into the core!) we don't need to use dma_pool.
These
> + * descriptors -- and associated data -- are cacheable.  We do need
to make
> + * sure their dcache entries are written back before handing them off
to
> + * the controller, though.
> + */
> +
> +static struct dw_desc *dwc_first_active(struct dw_dma_chan *dwc)
> +{
> +       return list_entry(dwc->active_list.next, struct dw_desc,
desc_node);
> +}
> +
> +static struct dw_desc *dwc_first_queued(struct dw_dma_chan *dwc)
> +{
> +       return list_entry(dwc->queue.next, struct dw_desc, desc_node);
> +}
> +
> +static struct dw_desc *dwc_desc_get(struct dw_dma_chan *dwc)
> +{
> +       struct dw_desc *desc, *_desc;
> +       struct dw_desc *ret = NULL;
> +       unsigned int i = 0;
> +
> +       spin_lock_bh(&dwc->lock);
> +       list_for_each_entry_safe(desc, _desc, &dwc->free_list,
desc_node) {
> +               if (async_tx_test_ack(&desc->txd)) {
> +                       list_del(&desc->desc_node);
> +                       ret = desc;
> +                       break;
> +               }
> +               dev_dbg(&dwc->chan.dev, "desc %p not ACKed\n", desc);
> +               i++;
> +       }
> +       spin_unlock_bh(&dwc->lock);
> +
> +       dev_vdbg(&dwc->chan.dev, "scanned %u descriptors on
freelist\n", i);
> +
> +       return ret;
> +}
> +
> +static void dwc_sync_desc_for_cpu(struct dw_dma_chan *dwc, struct
> dw_desc *desc)
> +{
> +       struct dw_desc  *child;
> +
> +       list_for_each_entry(child, &desc->txd.tx_list, desc_node)
> +               dma_sync_single_for_cpu(dwc->chan.dev.parent,
> +                               child->txd.phys, sizeof(child->lli),
> +                               DMA_TO_DEVICE);
> +       dma_sync_single_for_cpu(dwc->chan.dev.parent,
> +                       desc->txd.phys, sizeof(desc->lli),
> +                       DMA_TO_DEVICE);
> +}
> +
> +/*
> + * Move a descriptor, including any children, to the free list.
> + * `desc' must not be on any lists.
> + */
> +static void dwc_desc_put(struct dw_dma_chan *dwc, struct dw_desc
*desc)
> +{
> +       if (desc) {
> +               struct dw_desc *child;
> +
> +               dwc_sync_desc_for_cpu(dwc, desc);
> +
> +               spin_lock_bh(&dwc->lock);
> +               list_for_each_entry(child, &desc->txd.tx_list,
desc_node)
> +                       dev_vdbg(&dwc->chan.dev,
> +                                       "moving child desc %p to
freelist\n",
> +                                       child);
> +               list_splice_init(&desc->txd.tx_list, &dwc->free_list);
> +               dev_vdbg(&dwc->chan.dev, "moving desc %p to
freelist\n",
> desc); +               list_add(&desc->desc_node, &dwc->free_list);
> +               spin_unlock_bh(&dwc->lock);
> +       }
> +}
> +
> +/* Called with dwc->lock held and bh disabled */
> +static dma_cookie_t
> +dwc_assign_cookie(struct dw_dma_chan *dwc, struct dw_desc *desc)
> +{
> +       dma_cookie_t cookie = dwc->chan.cookie;
> +
> +       if (++cookie < 0)
> +               cookie = 1;
> +
> +       dwc->chan.cookie = cookie;
> +       desc->txd.cookie = cookie;
> +
> +       return cookie;
> +}
> +
>
+/*---------------------------------------------------------------------
-*/
> +
> +/* Called with dwc->lock held and bh disabled */
> +static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc
*first)
> +{
> +       struct dw_dma   *dw = to_dw_dma(dwc->chan.device);
> +
> +       /* ASSERT:  channel is idle */
> +       if (dma_readl(dw, CH_EN) & dwc->mask) {
> +               dev_err(&dwc->chan.dev,
> +                       "BUG: Attempted to start non-idle channel\n");
> +               dev_err(&dwc->chan.dev,
> +                       "  SAR: 0x%x DAR: 0x%x LLP: 0x%x CTL:
0x%x:%08x\n",
> +                       channel_readl(dwc, SAR),
> +                       channel_readl(dwc, DAR),
> +                       channel_readl(dwc, LLP),
> +                       channel_readl(dwc, CTL_HI),
> +                       channel_readl(dwc, CTL_LO));
> +
> +               /* The tasklet will hopefully advance the queue... */
> +               return;

Should not at this point an error status be returned 
so that it can be handled accordingly by dwc_dostart() caller?

> +       }
> +
> +       channel_writel(dwc, LLP, first->txd.phys);
> +       channel_writel(dwc, CTL_LO,
> +                       DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
> +       channel_writel(dwc, CTL_HI, 0);
> +       channel_set_bit(dw, CH_EN, dwc->mask);
> +}
> +
>
+/*---------------------------------------------------------------------
-*/
> +
> +static void
> +dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc
*desc)
> +{
> +       dma_async_tx_callback           callback;
> +       void                            *param;
> +       struct dma_async_tx_descriptor  *txd = &desc->txd;
> +
> +       dev_vdbg(&dwc->chan.dev, "descriptor %u complete\n",
txd->cookie);
> +
> +       dwc->completed = txd->cookie;
> +       callback = txd->callback;
> +       param = txd->callback_param;
> +
> +       dwc_sync_desc_for_cpu(dwc, desc);
> +       list_splice_init(&txd->tx_list, &dwc->free_list);
> +       list_move(&desc->desc_node, &dwc->free_list);
> +
> +       /*
> +        * The API requires that no submissions are done from a
> +        * callback, so we don't need to drop the lock here
> +        */
> +       if (callback)
> +               callback(param);
> +}
> +
> +static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan
*dwc)
> +{
> +       struct dw_desc *desc, *_desc;
> +       LIST_HEAD(list);
> +
> +       if (dma_readl(dw, CH_EN) & dwc->mask) {
> +               dev_err(&dwc->chan.dev,
> +                       "BUG: XFER bit set, but channel not idle!\n");
> +
> +               /* Try to continue after resetting the channel... */
> +               channel_clear_bit(dw, CH_EN, dwc->mask);
> +               while (dma_readl(dw, CH_EN) & dwc->mask)
> +                       cpu_relax();
> +       }
> +
> +       /*
> +        * Submit queued descriptors ASAP, i.e. before we go through
> +        * the completed ones.
> +        */
> +       if (!list_empty(&dwc->queue))
> +               dwc_dostart(dwc, dwc_first_queued(dwc));
> +       list_splice_init(&dwc->active_list, &list);
> +       list_splice_init(&dwc->queue, &dwc->active_list);
> +
> +       list_for_each_entry_safe(desc, _desc, &list, desc_node)
> +               dwc_descriptor_complete(dwc, desc);
> +}
> +
> +static void dwc_scan_descriptors(struct dw_dma *dw, struct
dw_dma_chan *dwc)
> +{
> +       dma_addr_t llp;
> +       struct dw_desc *desc, *_desc;
> +       struct dw_desc *child;
> +       u32 status_xfer;
> +
> +       /*
> +        * Clear block interrupt flag before scanning so that we don't
> +        * miss any, and read LLP before RAW_XFER to ensure it is
> +        * valid if we decide to scan the list.
> +        */
> +       dma_writel(dw, CLEAR.BLOCK, dwc->mask);
> +       llp = channel_readl(dwc, LLP);
> +       status_xfer = dma_readl(dw, RAW.XFER);
> +
> +       if (status_xfer & dwc->mask) {
> +               /* Everything we've submitted is done */
> +               dma_writel(dw, CLEAR.XFER, dwc->mask);
> +               dwc_complete_all(dw, dwc);
> +               return;
> +       }
> +
> +       dev_vdbg(&dwc->chan.dev, "scan_descriptors: llp=0x%x\n", llp);
> +
> +       list_for_each_entry_safe(desc, _desc, &dwc->active_list,
desc_node) {
> +               if (desc->lli.llp == llp)
> +                       /* This one is currently in progress */
> +                       return;
> +
> +               list_for_each_entry(child, &desc->txd.tx_list,
desc_node)
> +                       if (child->lli.llp == llp)
> +                               /* Currently in progress */
> +                               return;
> +
> +               /*
> +                * No descriptors so far seem to be in progress, i.e.
> +                * this one must be done.
> +                */
> +               dwc_descriptor_complete(dwc, desc);
> +       }
> +
> +       dev_err(&dwc->chan.dev,
> +               "BUG: All descriptors done, but channel not idle!\n");
> +
> +       /* Try to continue after resetting the channel... */
> +       channel_clear_bit(dw, CH_EN, dwc->mask);
> +       while (dma_readl(dw, CH_EN) & dwc->mask)
> +               cpu_relax();
> +
> +       if (!list_empty(&dwc->queue)) {
> +               dwc_dostart(dwc, dwc_first_queued(dwc));
> +               list_splice_init(&dwc->queue, &dwc->active_list);
> +       }
> +}
> +
> +static void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_lli *lli)
> +{
> +       dev_printk(KERN_CRIT, &dwc->chan.dev,
> +                       "  desc: s0x%x d0x%x l0x%x c0x%x:%x\n",
> +                       lli->sar, lli->dar, lli->llp,
> +                       lli->ctlhi, lli->ctllo);
> +}
> +
> +static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan
*dwc)
> +{
> +       struct dw_desc *bad_desc;
> +       struct dw_desc *child;
> +
> +       dwc_scan_descriptors(dw, dwc);
> +
> +       /*
> +        * The descriptor currently at the head of the active list is
> +        * borked. Since we don't have any way to report errors, we'll
> +        * just have to scream loudly and try to carry on.
> +        */
> +       bad_desc = dwc_first_active(dwc);
> +       list_del_init(&bad_desc->desc_node);
> +       list_splice_init(&dwc->queue, dwc->active_list.prev);
> +
> +       /* Clear the error flag and try to restart the controller */
> +       dma_writel(dw, CLEAR.ERROR, dwc->mask);
> +       if (!list_empty(&dwc->active_list))
> +               dwc_dostart(dwc, dwc_first_active(dwc));
> +
> +       /*
> +        * KERN_CRITICAL may seem harsh, but since this only happens
> +        * when someone submits a bad physical address in a
> +        * descriptor, we should consider ourselves lucky that the
> +        * controller flagged an error instead of scribbling over
> +        * random memory locations.
> +        */
> +       dev_printk(KERN_CRIT, &dwc->chan.dev,
> +                       "Bad descriptor submitted for DMA!\n");
> +       dev_printk(KERN_CRIT, &dwc->chan.dev,
> +                       "  cookie: %d\n", bad_desc->txd.cookie);
> +       dwc_dump_lli(dwc, &bad_desc->lli);
> +       list_for_each_entry(child, &bad_desc->txd.tx_list, desc_node)
> +               dwc_dump_lli(dwc, &child->lli);
> +
> +       /* Pretend the descriptor completed successfully */
> +       dwc_descriptor_complete(dwc, bad_desc);
> +}
> +
> +static void dw_dma_tasklet(unsigned long data)
> +{
> +       struct dw_dma *dw = (struct dw_dma *)data;
> +       struct dw_dma_chan *dwc;
> +       u32 status_block;
> +       u32 status_xfer;
> +       u32 status_err;
> +       int i;
> +
> +       status_block = dma_readl(dw, RAW.BLOCK);
> +       status_xfer = dma_readl(dw, RAW.BLOCK);
> +       status_err = dma_readl(dw, RAW.ERROR);
> +
> +       dev_vdbg(dw->dma.dev, "tasklet: status_block=%x
status_err=%x\n",
> +                       status_block, status_err);
> +
> +       for (i = 0; i < dw->dma.chancnt; i++) {
> +               dwc = &dw->chan[i];
> +               spin_lock(&dwc->lock);
> +               if (status_err & (1 << i))
> +                       dwc_handle_error(dw, dwc);
> +               else if ((status_block | status_xfer) & (1 << i))
> +                       dwc_scan_descriptors(dw, dwc);
> +               spin_unlock(&dwc->lock);
> +       }
> +
> +       /*
> +        * Re-enable interrupts. Block Complete interrupts are only
> +        * enabled if the INT_EN bit in the descriptor is set. This
> +        * will trigger a scan before the whole list is done.
> +        */
> +       channel_set_bit(dw, MASK.XFER, dw->all_chan_mask);
> +       channel_set_bit(dw, MASK.BLOCK, dw->all_chan_mask);
> +       channel_set_bit(dw, MASK.ERROR, dw->all_chan_mask);
> +}
> +
> +static irqreturn_t dw_dma_interrupt(int irq, void *dev_id)
> +{
> +       struct dw_dma *dw = dev_id;
> +       u32 status;
> +
> +       dev_vdbg(dw->dma.dev, "interrupt: status=0x%x\n",
> +                       dma_readl(dw, STATUS_INT));
> +
> +       /*
> +        * Just disable the interrupts. We'll turn them back on in the
> +        * softirq handler.
> +        */
> +       channel_clear_bit(dw, MASK.XFER, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);
> +
> +       status = dma_readl(dw, STATUS_INT);
> +       if (status) {
> +               dev_err(dw->dma.dev,
> +                       "BUG: Unexpected interrupts pending: 0x%x\n",
> +                       status);
> +
> +               /* Try to recover */
> +               channel_clear_bit(dw, MASK.XFER, (1 << 8) - 1);
> +               channel_clear_bit(dw, MASK.BLOCK, (1 << 8) - 1);
> +               channel_clear_bit(dw, MASK.SRC_TRAN, (1 << 8) - 1);
> +               channel_clear_bit(dw, MASK.DST_TRAN, (1 << 8) - 1);
> +               channel_clear_bit(dw, MASK.ERROR, (1 << 8) - 1);
> +       }
> +
> +       tasklet_schedule(&dw->tasklet);
> +
> +       return IRQ_HANDLED;
> +}
> +
>
+/*---------------------------------------------------------------------
-*/
> +
> +static dma_cookie_t dwc_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> +       struct dw_desc          *desc = txd_to_dw_desc(tx);
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(tx->chan);
> +       dma_cookie_t            cookie;
> +
> +       spin_lock_bh(&dwc->lock);
> +       cookie = dwc_assign_cookie(dwc, desc);
> +
> +       /*
> +        * REVISIT: We should attempt to chain as many descriptors as
> +        * possible, perhaps even appending to those already submitted
> +        * for DMA. But this is hard to do in a race-free manner.
> +        */
> +       if (list_empty(&dwc->active_list)) {
> +               dev_vdbg(&tx->chan->dev, "tx_submit: started %u\n",
> +                               desc->txd.cookie);
> +               dwc_dostart(dwc, desc);
> +               list_add_tail(&desc->desc_node, &dwc->active_list);
> +       } else {
> +               dev_vdbg(&tx->chan->dev, "tx_submit: queued %u\n",
> +                               desc->txd.cookie);
> +
> +               list_add_tail(&desc->desc_node, &dwc->queue);
> +       }
> +
> +       spin_unlock_bh(&dwc->lock);
> +
> +       return cookie;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest,
dma_addr_t src,
> +               size_t len, unsigned long flags)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       struct dw_desc          *desc;
> +       struct dw_desc          *first;
> +       struct dw_desc          *prev;
> +       size_t                  xfer_count;
> +       size_t                  offset;
> +       unsigned int            src_width;
> +       unsigned int            dst_width;
> +       u32                     ctllo;
> +
> +       dev_vdbg(&chan->dev, "prep_dma_memcpy d0x%x s0x%x l0x%zx
f0x%lx\n",
> +                       dest, src, len, flags);
> +
> +       if (unlikely(!len)) {
> +               dev_dbg(&chan->dev, "prep_dma_memcpy: length is
zero!\n");
> +               return NULL;
> +       }
> +
> +       /*
> +        * We can be a lot more clever here, but this should take care
> +        * of the most common optimization.
> +        */
> +       if (!((src | dest  | len) & 3))
> +               src_width = dst_width = 2;
> +       else if (!((src | dest | len) & 1))
> +               src_width = dst_width = 1;
> +       else
> +               src_width = dst_width = 0;
> +
> +       ctllo = DWC_DEFAULT_CTLLO
> +                       | DWC_CTLL_DST_WIDTH(dst_width)
> +                       | DWC_CTLL_SRC_WIDTH(src_width)
> +                       | DWC_CTLL_DST_INC
> +                       | DWC_CTLL_SRC_INC
> +                       | DWC_CTLL_FC_M2M;
> +       prev = first = NULL;
> +
> +       for (offset = 0; offset < len; offset += xfer_count <<
src_width) {
> +               xfer_count = min_t(size_t, (len - offset) >>
src_width,
> +                               DWC_MAX_COUNT);

Here it looks like the maximum xfer_count value can change - it depends
on src_width, 
so it may be different for different transactions.
Is that ok?

> +
> +               desc = dwc_desc_get(dwc);
> +               if (!desc)
> +                       goto err_desc_get;
> +
> +               desc->lli.sar = src + offset;
> +               desc->lli.dar = dest + offset;
> +               desc->lli.ctllo = ctllo;
> +               desc->lli.ctlhi = xfer_count;
> +
> +               if (!first) {
> +                       first = desc;
> +               } else {
> +                       prev->lli.llp = desc->txd.phys;
> +                       dma_sync_single_for_device(chan->dev.parent,
> +                                       prev->txd.phys,
sizeof(prev->lli),
> +                                       DMA_TO_DEVICE);
> +                       list_add_tail(&desc->desc_node,
> +                                       &first->txd.tx_list);
> +               }
> +               prev = desc;
> +       }
> +
> +
> +       if (flags & DMA_PREP_INTERRUPT)
> +               /* Trigger interrupt after last block */
> +               prev->lli.ctllo |= DWC_CTLL_INT_EN;
> +
> +       prev->lli.llp = 0;
> +       dma_sync_single_for_device(chan->dev.parent,
> +                       prev->txd.phys, sizeof(prev->lli),
> +                       DMA_TO_DEVICE);
> +
> +       first->txd.flags = flags;
> +
> +       return &first->txd;
> +
> +err_desc_get:
> +       dwc_desc_put(dwc, first);
> +       return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> +               unsigned int sg_len, enum dma_data_direction
direction,
> +               unsigned long flags)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       struct dw_dma_slave     *dws = dwc->dws;
> +       struct dw_desc          *prev;
> +       struct dw_desc          *first;
> +       u32                     ctllo;
> +       dma_addr_t              reg;
> +       unsigned int            reg_width;
> +       unsigned int            mem_width;
> +       unsigned int            i;
> +       struct scatterlist      *sg;
> +
> +       dev_vdbg(&chan->dev, "prep_dma_slave\n");
> +
> +       if (unlikely(!dws || !sg_len))
> +               return NULL;
> +
> +       reg_width = dws->slave.reg_width;
> +       prev = first = NULL;
> +
> +       sg_len = dma_map_sg(chan->dev.parent, sgl, sg_len, direction);
> +
> +       switch (direction) {
> +       case DMA_TO_DEVICE:
> +               ctllo = (DWC_DEFAULT_CTLLO
> +                               | DWC_CTLL_DST_WIDTH(reg_width)
> +                               | DWC_CTLL_DST_FIX
> +                               | DWC_CTLL_SRC_INC
> +                               | DWC_CTLL_FC_M2P);
> +               reg = dws->slave.tx_reg;
> +               for_each_sg(sgl, sg, sg_len, i) {
> +                       struct dw_desc  *desc;
> +                       u32             len;
> +                       u32             mem;
> +
> +                       desc = dwc_desc_get(dwc);
> +                       if (!desc) {
> +                               dev_err(&chan->dev,
> +                                       "not enough descriptors
available\n");
> +                               goto err_desc_get;
> +                       }
> +
> +                       mem = sg_phys(sg);
> +                       len = sg_dma_len(sg);
> +                       mem_width = 2;
> +                       if (unlikely(mem & 3 || len & 3))
> +                               mem_width = 0;
> +
> +                       desc->lli.sar = mem;
> +                       desc->lli.dar = reg;
> +                       desc->lli.ctllo = ctllo |
> DWC_CTLL_SRC_WIDTH(mem_width); +                       desc->lli.ctlhi
= len
> >> mem_width; +
> +                       if (!first) {
> +                               first = desc;
> +                       } else {
> +                               prev->lli.llp = desc->txd.phys;
> +
dma_sync_single_for_device(chan->dev.parent,
> +                                               prev->txd.phys,
> +                                               sizeof(prev->lli),
> +                                               DMA_TO_DEVICE);
> +                               list_add_tail(&desc->desc_node,
> +                                               &first->txd.tx_list);
> +                       }
> +                       prev = desc;
> +               }
> +               break;
> +       case DMA_FROM_DEVICE:
> +               ctllo = (DWC_DEFAULT_CTLLO
> +                               | DWC_CTLL_SRC_WIDTH(reg_width)
> +                               | DWC_CTLL_DST_INC
> +                               | DWC_CTLL_SRC_FIX
> +                               | DWC_CTLL_FC_P2M);
> +
> +               reg = dws->slave.rx_reg;
> +               for_each_sg(sgl, sg, sg_len, i) {
> +                       struct dw_desc  *desc;
> +                       u32             len;
> +                       u32             mem;
> +
> +                       desc = dwc_desc_get(dwc);
> +                       if (!desc) {
> +                               dev_err(&chan->dev,
> +                                       "not enough descriptors
available\n");
> +                               goto err_desc_get;
> +                       }
> +
> +                       mem = sg_phys(sg);
> +                       len = sg_dma_len(sg);
> +                       mem_width = 2;
> +                       if (unlikely(mem & 3 || len & 3))
> +                               mem_width = 0;
> +
> +                       desc->lli.sar = reg;
> +                       desc->lli.dar = mem;
> +                       desc->lli.ctllo = ctllo |
> DWC_CTLL_DST_WIDTH(mem_width); +                       desc->lli.ctlhi
= len
> >> reg_width; +
> +                       if (!first) {
> +                               first = desc;
> +                       } else {
> +                               prev->lli.llp = desc->txd.phys;
> +
dma_sync_single_for_device(chan->dev.parent,
> +                                               prev->txd.phys,
> +                                               sizeof(prev->lli),
> +                                               DMA_TO_DEVICE);
> +                               list_add_tail(&desc->desc_node,
> +                                               &first->txd.tx_list);
> +                       }
> +                       prev = desc;
> +               }
> +               break;
> +       default:
> +               return NULL;
> +       }
> +
> +       if (flags & DMA_PREP_INTERRUPT)
> +               /* Trigger interrupt after last block */
> +               prev->lli.ctllo |= DWC_CTLL_INT_EN;
> +
> +       prev->lli.llp = 0;
> +       dma_sync_single_for_device(chan->dev.parent,
> +                       prev->txd.phys, sizeof(prev->lli),
> +                       DMA_TO_DEVICE);
> +
> +       return &first->txd;
> +
> +err_desc_get:
> +       dwc_desc_put(dwc, first);
> +       return NULL;
> +}
> +
> +static void dwc_terminate_all(struct dma_chan *chan)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       struct dw_dma           *dw = to_dw_dma(chan->device);
> +       struct dw_desc          *desc, *_desc;
> +       LIST_HEAD(list);
> +
> +       /*
> +        * This is only called when something went wrong elsewhere, so
> +        * we don't really care about the data. Just disable the
> +        * channel. We still have to poll the channel enable bit due
> +        * to AHB/HSB limitations.
> +        */
> +       spin_lock_bh(&dwc->lock);
> +
> +       channel_clear_bit(dw, CH_EN, dwc->mask);
> +
> +       while (dma_readl(dw, CH_EN) & dwc->mask)
> +               cpu_relax();
> +
> +       /* active_list entries will end up before queued entries */
> +       list_splice_init(&dwc->queue, &list);
> +       list_splice_init(&dwc->active_list, &list);
> +
> +       spin_unlock_bh(&dwc->lock);
> +
> +       /* Flush all pending and queued descriptors */
> +       list_for_each_entry_safe(desc, _desc, &list, desc_node)
> +               dwc_descriptor_complete(dwc, desc);
> +}
> +
> +static enum dma_status
> +dwc_is_tx_complete(struct dma_chan *chan,
> +               dma_cookie_t cookie,
> +               dma_cookie_t *done, dma_cookie_t *used)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       dma_cookie_t            last_used;
> +       dma_cookie_t            last_complete;
> +       int                     ret;
> +
> +       last_complete = dwc->completed;
> +       last_used = chan->cookie;
> +
> +       ret = dma_async_is_complete(cookie, last_complete, last_used);
> +       if (ret != DMA_SUCCESS) {
> +               dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
> +
> +               last_complete = dwc->completed;
> +               last_used = chan->cookie;
> +
> +               ret = dma_async_is_complete(cookie, last_complete,
last_used);
> +       }
> +
> +       if (done)
> +               *done = last_complete;
> +       if (used)
> +               *used = last_used;
> +
> +       return ret;
> +}
> +
> +static void dwc_issue_pending(struct dma_chan *chan)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +
> +       spin_lock_bh(&dwc->lock);
> +       if (!list_empty(&dwc->queue))
> +               dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
> +       spin_unlock_bh(&dwc->lock);
> +}
> +
> +static int dwc_alloc_chan_resources(struct dma_chan *chan,
> +               struct dma_client *client)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       struct dw_dma           *dw = to_dw_dma(chan->device);
> +       struct dw_desc          *desc;
> +       struct dma_slave        *slave;
> +       struct dw_dma_slave     *dws;
> +       int                     i;
> +       u32                     cfghi;
> +       u32                     cfglo;
> +
> +       dev_vdbg(&chan->dev, "alloc_chan_resources\n");
> +
> +       /* Channels doing slave DMA can only handle one client. */
> +       if (dwc->dws || client->slave) {
> +               if (dma_chan_is_in_use(chan))
> +                       return -EBUSY;
> +       }
> +
> +       /* ASSERT:  channel is idle */
> +       if (dma_readl(dw, CH_EN) & dwc->mask) {
> +               dev_dbg(&chan->dev, "DMA channel not idle?\n");
> +               return -EIO;
> +       }
> +
> +       dwc->completed = chan->cookie = 1;
> +
> +       cfghi = DWC_CFGH_FIFO_MODE;
> +       cfglo = 0;
> +
> +       slave = client->slave;
> +       if (slave) {
> +               /*
> +                * We need controller-specific data to set up slave
> +                * transfers.
> +                */
> +               BUG_ON(!slave->dma_dev || slave->dma_dev !=
dw->dma.dev);
> +
> +               dws = container_of(slave, struct dw_dma_slave, slave);
> +
> +               dwc->dws = dws;
> +               cfghi = dws->cfg_hi;
> +               cfglo = dws->cfg_lo;
> +       } else {
> +               dwc->dws = NULL;
> +       }
> +
> +       channel_writel(dwc, CFG_LO, cfglo);
> +       channel_writel(dwc, CFG_HI, cfghi);
> +
> +       /*
> +        * NOTE: some controllers may have additional features that we
> +        * need to initialize here, like "scatter-gather" (which
> +        * doesn't mean what you think it means), and status
writeback.
> +        */
> +
> +       spin_lock_bh(&dwc->lock);
> +       i = dwc->descs_allocated;
> +       while (dwc->descs_allocated < NR_DESCS_PER_CHANNEL) {
> +               spin_unlock_bh(&dwc->lock);
> +
> +               desc = kzalloc(sizeof(struct dw_desc), GFP_KERNEL);
> +               if (!desc) {
> +                       dev_info(&chan->dev,
> +                               "only allocated %d descriptors\n", i);
> +                       spin_lock_bh(&dwc->lock);
> +                       break;
> +               }
> +
> +               dma_async_tx_descriptor_init(&desc->txd, chan);
> +               desc->txd.tx_submit = dwc_tx_submit;
> +               desc->txd.flags = DMA_CTRL_ACK;
> +               INIT_LIST_HEAD(&desc->txd.tx_list);
> +               desc->txd.phys = dma_map_single(chan->dev.parent,
&desc->lli,
> +                               sizeof(desc->lli), DMA_TO_DEVICE);
> +               dwc_desc_put(dwc, desc);
> +
> +               spin_lock_bh(&dwc->lock);
> +               i = ++dwc->descs_allocated;
> +       }
> +
> +       /* Enable interrupts */
> +       channel_set_bit(dw, MASK.XFER, dwc->mask);
> +       channel_set_bit(dw, MASK.BLOCK, dwc->mask);
> +       channel_set_bit(dw, MASK.ERROR, dwc->mask);
> +
> +       spin_unlock_bh(&dwc->lock);
> +
> +       dev_dbg(&chan->dev,
> +               "alloc_chan_resources allocated %d descriptors\n", i);
> +
> +       return i;
> +}
> +
> +static void dwc_free_chan_resources(struct dma_chan *chan)
> +{
> +       struct dw_dma_chan      *dwc = to_dw_dma_chan(chan);
> +       struct dw_dma           *dw = to_dw_dma(chan->device);
> +       struct dw_desc          *desc, *_desc;
> +       LIST_HEAD(list);
> +
> +       dev_dbg(&chan->dev, "free_chan_resources (descs
allocated=%u)\n",
> +                       dwc->descs_allocated);
> +
> +       /* ASSERT:  channel is idle */
> +       BUG_ON(!list_empty(&dwc->active_list));
> +       BUG_ON(!list_empty(&dwc->queue));
> +       BUG_ON(dma_readl(to_dw_dma(chan->device), CH_EN) & dwc->mask);
> +
> +       spin_lock_bh(&dwc->lock);
> +       list_splice_init(&dwc->free_list, &list);
> +       dwc->descs_allocated = 0;
> +       dwc->dws = NULL;
> +
> +       /* Disable interrupts */
> +       channel_clear_bit(dw, MASK.XFER, dwc->mask);
> +       channel_clear_bit(dw, MASK.BLOCK, dwc->mask);
> +       channel_clear_bit(dw, MASK.ERROR, dwc->mask);
> +
> +       spin_unlock_bh(&dwc->lock);
> +
> +       list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +               dev_vdbg(&chan->dev, "  freeing descriptor %p\n",
desc);
> +               dma_unmap_single(chan->dev.parent, desc->txd.phys,
> +                               sizeof(desc->lli), DMA_TO_DEVICE);
> +               kfree(desc);
> +       }
> +
> +       dev_vdbg(&chan->dev, "free_chan_resources done\n");
> +}
> +
>
+/*---------------------------------------------------------------------
-*/
> +
> +static void dw_dma_off(struct dw_dma *dw)
> +{
> +       dma_writel(dw, CFG, 0);
> +
> +       channel_clear_bit(dw, MASK.XFER, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.SRC_TRAN, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.DST_TRAN, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);
> +
> +       while (dma_readl(dw, CFG) & DW_CFG_DMA_EN)
> +               cpu_relax();
> +}
> +
> +static int __init dw_probe(struct platform_device *pdev)
> +{
> +       struct dw_dma_platform_data *pdata;
> +       struct resource         *io;
> +       struct dw_dma           *dw;
> +       size_t                  size;
> +       int                     irq;
> +       int                     err;
> +       int                     i;
> +
> +       pdata = pdev->dev.platform_data;
> +       if (!pdata || pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS)
> +               return -EINVAL;
> +
> +       io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!io)
> +               return -EINVAL;
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0)
> +               return irq;
> +
> +       size = sizeof(struct dw_dma);
> +       size += pdata->nr_channels * sizeof(struct dw_dma_chan);
> +       dw = kzalloc(size, GFP_KERNEL);
> +       if (!dw)
> +               return -ENOMEM;
> +
> +       if (!request_mem_region(io->start, DW_REGLEN,
> pdev->dev.driver->name)) { +               err = -EBUSY;
> +               goto err_kfree;
> +       }
> +
> +       memset(dw, 0, sizeof *dw);
> +
> +       dw->regs = ioremap(io->start, DW_REGLEN);
> +       if (!dw->regs) {
> +               err = -ENOMEM;
> +               goto err_release_r;
> +       }
> +
> +       dw->clk = clk_get(&pdev->dev, "hclk");
> +       if (IS_ERR(dw->clk)) {
> +               err = PTR_ERR(dw->clk);
> +               goto err_clk;
> +       }
> +       clk_enable(dw->clk);
> +
> +       /* force dma off, just in case */
> +       dw_dma_off(dw);
> +
> +       err = request_irq(irq, dw_dma_interrupt, 0, "dw_dmac", dw);
> +       if (err)
> +               goto err_irq;
> +
> +       platform_set_drvdata(pdev, dw);
> +
> +       tasklet_init(&dw->tasklet, dw_dma_tasklet, (unsigned long)dw);
> +
> +       dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
> +
> +       INIT_LIST_HEAD(&dw->dma.channels);
> +       for (i = 0; i < pdata->nr_channels; i++, dw->dma.chancnt++) {
> +               struct dw_dma_chan      *dwc = &dw->chan[i];
> +
> +               dwc->chan.device = &dw->dma;
> +               dwc->chan.cookie = dwc->completed = 1;
> +               dwc->chan.chan_id = i;
> +               list_add_tail(&dwc->chan.device_node,
&dw->dma.channels);
> +
> +               dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
> +               spin_lock_init(&dwc->lock);
> +               dwc->mask = 1 << i;
> +
> +               INIT_LIST_HEAD(&dwc->active_list);
> +               INIT_LIST_HEAD(&dwc->queue);
> +               INIT_LIST_HEAD(&dwc->free_list);
> +
> +               channel_clear_bit(dw, CH_EN, dwc->mask);
> +       }
> +
> +       /* Clear/disable all interrupts on all channels. */
> +       dma_writel(dw, CLEAR.XFER, dw->all_chan_mask);
> +       dma_writel(dw, CLEAR.BLOCK, dw->all_chan_mask);
> +       dma_writel(dw, CLEAR.SRC_TRAN, dw->all_chan_mask);
> +       dma_writel(dw, CLEAR.DST_TRAN, dw->all_chan_mask);
> +       dma_writel(dw, CLEAR.ERROR, dw->all_chan_mask);
> +
> +       channel_clear_bit(dw, MASK.XFER, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.BLOCK, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.SRC_TRAN, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.DST_TRAN, dw->all_chan_mask);
> +       channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);
> +
> +       dma_cap_set(DMA_MEMCPY, dw->dma.cap_mask);
> +       dma_cap_set(DMA_SLAVE, dw->dma.cap_mask);
> +       dw->dma.dev = &pdev->dev;
> +       dw->dma.device_alloc_chan_resources =
dwc_alloc_chan_resources;
> +       dw->dma.device_free_chan_resources = dwc_free_chan_resources;
> +
> +       dw->dma.device_prep_dma_memcpy = dwc_prep_dma_memcpy;
> +
> +       dw->dma.device_prep_slave_sg = dwc_prep_slave_sg;
> +       dw->dma.device_terminate_all = dwc_terminate_all;
> +
> +       dw->dma.device_is_tx_complete = dwc_is_tx_complete;
> +       dw->dma.device_issue_pending = dwc_issue_pending;
> +
> +       dma_writel(dw, CFG, DW_CFG_DMA_EN);
> +
> +       printk(KERN_INFO "%s: DesignWare DMA Controller, %d
channels\n",
> +                       pdev->dev.bus_id, dw->dma.chancnt);
> +
> +       dma_async_device_register(&dw->dma);
> +
> +       return 0;
> +
> +err_irq:
> +       clk_disable(dw->clk);
> +       clk_put(dw->clk);
> +err_clk:
> +       iounmap(dw->regs);
> +       dw->regs = NULL;
> +err_release_r:
> +       release_resource(io);
> +err_kfree:
> +       kfree(dw);
> +       return err;
> +}

This driver does not perform any self-test during initialization.
What about adding some initial HW checking?

> +
> +static int __exit dw_remove(struct platform_device *pdev)
> +{
> +       struct dw_dma           *dw = platform_get_drvdata(pdev);
> +       struct dw_dma_chan      *dwc, *_dwc;
> +       struct resource         *io;
> +
> +       dw_dma_off(dw);
> +       dma_async_device_unregister(&dw->dma);
> +
> +       free_irq(platform_get_irq(pdev, 0), dw);
> +       tasklet_kill(&dw->tasklet);
> +
> +       list_for_each_entry_safe(dwc, _dwc, &dw->dma.channels,
> +                       chan.device_node) {
> +               list_del(&dwc->chan.device_node);
> +               channel_clear_bit(dw, CH_EN, dwc->mask);
> +       }
> +
> +       clk_disable(dw->clk);
> +       clk_put(dw->clk);
> +
> +       iounmap(dw->regs);
> +       dw->regs = NULL;
> +
> +       io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       release_mem_region(io->start, DW_REGLEN);
> +
> +       kfree(dw);
> +
> +       return 0;
> +}
> +
> +static void dw_shutdown(struct platform_device *pdev)
> +{
> +       struct dw_dma   *dw = platform_get_drvdata(pdev);
> +
> +       dw_dma_off(platform_get_drvdata(pdev));
> +       clk_disable(dw->clk);
> +}
> +
> +static int dw_suspend_late(struct platform_device *pdev, pm_message_t
mesg)
> +{
> +       struct dw_dma   *dw = platform_get_drvdata(pdev);
> +
> +       dw_dma_off(platform_get_drvdata(pdev));
> +       clk_disable(dw->clk);
> +       return 0;
> +}
> +
> +static int dw_resume_early(struct platform_device *pdev)
> +{
> +       struct dw_dma   *dw = platform_get_drvdata(pdev);
> +
> +       clk_enable(dw->clk);
> +       dma_writel(dw, CFG, DW_CFG_DMA_EN);
> +       return 0;
> +
> +}
> +
> +static struct platform_driver dw_driver = {
> +       .remove         = __exit_p(dw_remove),
> +       .shutdown       = dw_shutdown,
> +       .suspend_late   = dw_suspend_late,
> +       .resume_early   = dw_resume_early,
> +       .driver = {
> +               .name   = "dw_dmac",
> +       },
> +};
> +
> +static int __init dw_init(void)
> +{
> +       return platform_driver_probe(&dw_driver, dw_probe);
> +}
> +module_init(dw_init);
> +
> +static void __exit dw_exit(void)
> +{
> +       platform_driver_unregister(&dw_driver);
> +}
> +module_exit(dw_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
> +MODULE_AUTHOR("Haavard Skinnemoen <haavard.skinnemoen@atmel.com>");
> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> new file mode 100644
> index 0000000..119e65b
> --- /dev/null
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -0,0 +1,224 @@
> +/*
> + * Driver for the Synopsys DesignWare AHB DMA Controller
> + *
> + * Copyright (C) 2005-2007 Atmel Corporation
> + *
> + * 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.
> + */
> +
> +#include <linux/dw_dmac.h>
> +
> +#define DW_DMA_MAX_NR_CHANNELS 8
> +
> +/*
> + * Redefine this macro to handle differences between 32- and 64-bit
> + * addressing, big vs. little endian, etc.
> + */
> +#define DW_REG(name)           u32 name; u32 __pad_##name
> +
> +/* Hardware register definitions. */
> +struct dw_dma_chan_regs {
> +       DW_REG(SAR);            /* Source Address Register */
> +       DW_REG(DAR);            /* Destination Address Register */
> +       DW_REG(LLP);            /* Linked List Pointer */
> +       u32     CTL_LO;         /* Control Register Low */
> +       u32     CTL_HI;         /* Control Register High */
> +       DW_REG(SSTAT);
> +       DW_REG(DSTAT);
> +       DW_REG(SSTATAR);
> +       DW_REG(DSTATAR);
> +       u32     CFG_LO;         /* Configuration Register Low */
> +       u32     CFG_HI;         /* Configuration Register High */
> +       DW_REG(SGR);
> +       DW_REG(DSR);
> +};
> +
> +struct dw_dma_irq_regs {
> +       DW_REG(XFER);
> +       DW_REG(BLOCK);
> +       DW_REG(SRC_TRAN);
> +       DW_REG(DST_TRAN);
> +       DW_REG(ERROR);
> +};
> +
> +struct dw_dma_regs {
> +       /* per-channel registers */
> +       struct dw_dma_chan_regs CHAN[DW_DMA_MAX_NR_CHANNELS];
> +
> +       /* irq handling */
> +       struct dw_dma_irq_regs  RAW;            /* r */
> +       struct dw_dma_irq_regs  STATUS;         /* r (raw & mask) */
> +       struct dw_dma_irq_regs  MASK;           /* rw (set = irq
enabled) */
> +       struct dw_dma_irq_regs  CLEAR;          /* w (ack, affects
"raw") */
> +
> +       DW_REG(STATUS_INT);                     /* r */
> +
> +       /* software handshaking */
> +       DW_REG(REQ_SRC);
> +       DW_REG(REQ_DST);
> +       DW_REG(SGL_REQ_SRC);
> +       DW_REG(SGL_REQ_DST);
> +       DW_REG(LAST_SRC);
> +       DW_REG(LAST_DST);
> +
> +       /* miscellaneous */
> +       DW_REG(CFG);
> +       DW_REG(CH_EN);
> +       DW_REG(ID);
> +       DW_REG(TEST);
> +
> +       /* optional encoded params, 0x3c8..0x3 */
> +};
> +
> +/* Bitfields in CTL_LO */
> +#define DWC_CTLL_INT_EN                (1 << 0)        /* irqs
enabled? */
> +#define DWC_CTLL_DST_WIDTH(n)  ((n)<<1)        /* bytes per element
*/
> +#define DWC_CTLL_SRC_WIDTH(n)  ((n)<<4)
> +#define DWC_CTLL_DST_INC       (0<<7)          /* DAR update/not */
> +#define DWC_CTLL_DST_DEC       (1<<7)
> +#define DWC_CTLL_DST_FIX       (2<<7)
> +#define DWC_CTLL_SRC_INC       (0<<7)          /* SAR update/not */
> +#define DWC_CTLL_SRC_DEC       (1<<9)
> +#define DWC_CTLL_SRC_FIX       (2<<9)
> +#define DWC_CTLL_DST_MSIZE(n)  ((n)<<11)       /* burst, #elements */
> +#define DWC_CTLL_SRC_MSIZE(n)  ((n)<<14)
> +#define DWC_CTLL_S_GATH_EN     (1 << 17)       /* src gather, !FIX */
> +#define DWC_CTLL_D_SCAT_EN     (1 << 18)       /* dst scatter, !FIX
*/
> +#define DWC_CTLL_FC_M2M                (0 << 20)       /* mem-to-mem
*/
> +#define DWC_CTLL_FC_M2P                (1 << 20)       /*
mem-to-periph */
> +#define DWC_CTLL_FC_P2M                (2 << 20)       /*
periph-to-mem */
> +#define DWC_CTLL_FC_P2P                (3 << 20)       /*
periph-to-periph */
> +/* plus 4 transfer types for peripheral-as-flow-controller */
> +#define DWC_CTLL_DMS(n)                ((n)<<23)       /* dst master
select
> */ +#define DWC_CTLL_SMS(n)                ((n)<<25)       /* src
master
> select */ +#define DWC_CTLL_LLP_D_EN      (1 << 27)       /* dest
block chain
> */ +#define DWC_CTLL_LLP_S_EN      (1 << 28)       /* src block chain
*/
> +
> +/* Bitfields in CTL_HI */
> +#define DWC_CTLH_DONE          0x00001000
> +#define DWC_CTLH_BLOCK_TS_MASK 0x00000fff
> +
> +/* Bitfields in CFG_LO. Platform-configurable bits are in
<linux/dw_dmac.h>
> */ +#define DWC_CFGL_CH_SUSP       (1 << 8)        /* pause xfer */
> +#define DWC_CFGL_FIFO_EMPTY    (1 << 9)        /* pause xfer */
> +#define DWC_CFGL_HS_DST                (1 << 10)       /* handshake
w/dst */
> +#define DWC_CFGL_HS_SRC                (1 << 11)       /* handshake
w/src */
> +#define DWC_CFGL_MAX_BURST(x)  ((x) << 20)
> +#define DWC_CFGL_RELOAD_SAR    (1 << 30)
> +#define DWC_CFGL_RELOAD_DAR    (1 << 31)
> +
> +/* Bitfields in CFG_HI. Platform-configurable bits are in
<linux/dw_dmac.h>
> */ +#define DWC_CFGH_DS_UPD_EN     (1 << 5)
> +#define DWC_CFGH_SS_UPD_EN     (1 << 6)
> +
> +/* Bitfields in SGR */
> +#define DWC_SGR_SGI(x)         ((x) << 0)
> +#define DWC_SGR_SGC(x)         ((x) << 20)
> +
> +/* Bitfields in DSR */
> +#define DWC_DSR_DSI(x)         ((x) << 0)
> +#define DWC_DSR_DSC(x)         ((x) << 20)
> +
> +/* Bitfields in CFG */
> +#define DW_CFG_DMA_EN          (1 << 0)
> +
> +#define DW_REGLEN              0x400
> +
> +struct dw_dma_chan {
> +       struct dma_chan         chan;
> +       void __iomem            *ch_regs;
> +       u8                      mask;
> +
> +       spinlock_t              lock;
> +
> +       /* these other elements are all protected by lock */
> +       dma_cookie_t            completed;
> +       struct list_head        active_list;
> +       struct list_head        queue;
> +       struct list_head        free_list;
> +
> +       struct dw_dma_slave     *dws;
> +
> +       unsigned int            descs_allocated;
> +};
> +
> +static inline struct dw_dma_chan_regs __iomem *
> +__dwc_regs(struct dw_dma_chan *dwc)
> +{
> +       return dwc->ch_regs;
> +}
> +
> +#define channel_readl(dwc, name) \
> +       __raw_readl(&(__dwc_regs(dwc)->name))
> +#define channel_writel(dwc, name, val) \
> +       __raw_writel((val), &(__dwc_regs(dwc)->name))
> +
> +static inline struct dw_dma_chan *to_dw_dma_chan(struct dma_chan
*chan)
> +{
> +       return container_of(chan, struct dw_dma_chan, chan);
> +}
> +
> +
> +struct dw_dma {
> +       struct dma_device       dma;
> +       void __iomem            *regs;
> +       struct tasklet_struct   tasklet;
> +       struct clk              *clk;
> +
> +       u8                      all_chan_mask;
> +
> +       struct dw_dma_chan      chan[0];
> +};
> +
> +static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma
*dw)
> +{
> +       return dw->regs;
> +}
> +
> +#define dma_readl(dw, name) \
> +       __raw_readl(&(__dw_regs(dw)->name))
> +#define dma_writel(dw, name, val) \
> +       __raw_writel((val), &(__dw_regs(dw)->name))
> +
> +#define channel_set_bit(dw, reg, mask) \
> +       dma_writel(dw, reg, ((mask) << 8) | (mask))
> +#define channel_clear_bit(dw, reg, mask) \
> +       dma_writel(dw, reg, ((mask) << 8) | 0)
> +
> +static inline struct dw_dma *to_dw_dma(struct dma_device *ddev)
> +{
> +       return container_of(ddev, struct dw_dma, dma);
> +}
> +
> +/* LLI == Linked List Item; a.k.a. DMA block descriptor */
> +struct dw_lli {
> +       /* values that are not changed by hardware */
> +       dma_addr_t      sar;
> +       dma_addr_t      dar;
> +       dma_addr_t      llp;            /* chain to next lli */
> +       u32             ctllo;
> +       /* values that may get written back: */
> +       u32             ctlhi;
> +       /* sstat and dstat can snapshot peripheral register state.
> +        * silicon config may discard either or both...
> +        */
> +       u32             sstat;
> +       u32             dstat;
> +};
> +
> +struct dw_desc {
> +       /* FIRST values the hardware uses */
> +       struct dw_lli                   lli;
> +
> +       /* THEN values for driver housekeeping */
> +       struct list_head                desc_node;
> +       struct dma_async_tx_descriptor  txd;
> +};
> +
> +static inline struct dw_desc *
> +txd_to_dw_desc(struct dma_async_tx_descriptor *txd)
> +{
> +       return container_of(txd, struct dw_desc, txd);
> +}
> diff --git a/include/asm-avr32/arch-at32ap/at32ap700x.h
> b/include/asm-avr32/arch-at32ap/at32ap700x.h
> index 31e48b0..d18a305 100644
> --- a/include/asm-avr32/arch-at32ap/at32ap700x.h
> +++ b/include/asm-avr32/arch-at32ap/at32ap700x.h
> @@ -30,4 +30,20 @@
>  #define GPIO_PIN_PD(N) (GPIO_PIOD_BASE + (N))
>  #define GPIO_PIN_PE(N) (GPIO_PIOE_BASE + (N))
> 
> +
> +/*
> + * DMAC peripheral hardware handshaking interfaces, used with dw_dmac
> + */
> +#define DMAC_MCI_RX            0
> +#define DMAC_MCI_TX            1
> +#define DMAC_DAC_TX            2
> +#define DMAC_AC97_A_RX         3
> +#define DMAC_AC97_A_TX         4
> +#define DMAC_AC97_B_RX         5
> +#define DMAC_AC97_B_TX         6
> +#define DMAC_DMAREQ_0          7
> +#define DMAC_DMAREQ_1          8
> +#define DMAC_DMAREQ_2          9
> +#define DMAC_DMAREQ_3          10
> +
>  #endif /* __ASM_ARCH_AT32AP700X_H__ */
> diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
> new file mode 100644
> index 0000000..04d217b
> --- /dev/null
> +++ b/include/linux/dw_dmac.h
> @@ -0,0 +1,62 @@
> +/*
> + * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
> + * AVR32 systems.)
> + *
> + * Copyright (C) 2007 Atmel Corporation
> + *
> + * 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.
> + */
> +#ifndef DW_DMAC_H
> +#define DW_DMAC_H
> +
> +#include <linux/dmaengine.h>
> +
> +/**
> + * struct dw_dma_platform_data - Controller configuration parameters
> + * @nr_channels: Number of channels supported by hardware (max 8)
> + */
> +struct dw_dma_platform_data {
> +       unsigned int    nr_channels;
> +};
> +
> +/**
> + * struct dw_dma_slave - Controller-specific information about a
slave
> + * @slave: Generic information about the slave
> + * @ctl_lo: Platform-specific initializer for the CTL_LO register
> + * @cfg_hi: Platform-specific initializer for the CFG_HI register
> + * @cfg_lo: Platform-specific initializer for the CFG_LO register
> + */
> +struct dw_dma_slave {
> +       struct dma_slave        slave;
> +       u32                     cfg_hi;
> +       u32                     cfg_lo;
> +};
> +
> +/* Platform-configurable bits in CFG_HI */
> +#define DWC_CFGH_FCMODE                (1 << 0)
> +#define DWC_CFGH_FIFO_MODE     (1 << 1)
> +#define DWC_CFGH_PROTCTL(x)    ((x) << 2)
> +#define DWC_CFGH_SRC_PER(x)    ((x) << 7)
> +#define DWC_CFGH_DST_PER(x)    ((x) << 11)
> +
> +/* Platform-configurable bits in CFG_LO */
> +#define DWC_CFGL_PRIO(x)       ((x) << 5)      /* priority */
> +#define DWC_CFGL_LOCK_CH_XFER  (0 << 12)       /* scope of LOCK_CH */
> +#define DWC_CFGL_LOCK_CH_BLOCK (1 << 12)
> +#define DWC_CFGL_LOCK_CH_XACT  (2 << 12)
> +#define DWC_CFGL_LOCK_BUS_XFER (0 << 14)       /* scope of LOCK_BUS
*/
> +#define DWC_CFGL_LOCK_BUS_BLOCK        (1 << 14)
> +#define DWC_CFGL_LOCK_BUS_XACT (2 << 14)
> +#define DWC_CFGL_LOCK_CH       (1 << 15)       /* channel lockout */
> +#define DWC_CFGL_LOCK_BUS      (1 << 16)       /* busmaster lockout
*/
> +#define DWC_CFGL_HS_DST_POL    (1 << 18)       /* dst handshake
active low */
> +#define DWC_CFGL_HS_SRC_POL    (1 << 19)       /* src handshake
active low */
> +
> +static inline struct dw_dma_slave *to_dw_dma_slave(struct dma_slave
*slave)
> +{
> +       return container_of(slave, struct dw_dma_slave, slave);
> +}
> +
> +#endif /* DW_DMAC_H */
> --
> 1.5.5.4

Regards,
Maciej

^ permalink raw reply

* Re: [PATCH v4 0/6] dmaengine/mmc: DMA slave interface and two new drivers
From: Haavard Skinnemoen @ 2008-07-04 15:13 UTC (permalink / raw)
  To: Dan Williams
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	shannon.nelson, David Brownell
In-Reply-To: <e9c3a7c20807031806u23304279p33ed3467ba0e5a7f@mail.gmail.com>

"Dan Williams" <dan.j.williams@intel.com> wrote:
> On Thu, Jun 26, 2008 at 6:23 AM, Haavard Skinnemoen
> <haavard.skinnemoen@atmel.com> wrote:
> >      dmaengine: Add dma_client parameter to device_alloc_chan_resources
> 
> Applied.  I fixed it up for fsldma and mv_xor.

Thanks.

> >      dmaengine: Add dma_chan_is_in_use() function
> 
> I applied the chan->client_count patch that we talked about.

Ok, I've updated the dw_dmac driver.

> >      dmaengine: Add slave DMA interface
> 
> There were some comments to the change log and other fixes, so I'll
> wait for v5 of this patch.

Will post v5 right after I finish typing this.

> >      dmaengine: Make DMA Engine menu visible for AVR32 users
> 
> Applied the "remove arch dependency in drivers/dma/Kconfig" instead.

Ok.

> >      dmaengine: Driver for the Synopsys DesignWare DMA controller
> >      Atmel MCI: Driver for Atmel on-chip MMC controllers
> 
> I will wait for v5 on these as well for the chan->client_count fixups
> and a response to the dma_unmap situation.

I'll send you v5 of the dw_dmac driver. The MMC driver should go in via
Pierre.

Thanks,

Haavard

^ permalink raw reply

* Re: dmaengine skip unmap (was: Re: [PATCH v4 5/6] dmaengine: Driver for the Synopsys DesignWare DMA controller)
From: Haavard Skinnemoen @ 2008-07-04 14:47 UTC (permalink / raw)
  To: Dan Williams
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	Nelson, Shannon, David Brownell
In-Reply-To: <1215132023.23470.15.camel@dwillia2-linux.ch.intel.com>

Dan Williams <dan.j.williams@intel.com> wrote:
> The one thing that stands out is that this driver does not unmap the
> source or destination buffers (and I now notice that fsldma is not doing
> this either, hmm...).  Yes, it is a no-op on avr32, for now, but the
> dma-mapping-api assumes that dma_map is always paired with dma_unmap.  I
> remember we discussed this earlier and that discussion inspired the
> patch below.  The end result is that dw_dmac can try to automatically
> dma_unmap the buffers unless an intelligent client, like the mmc driver,
> has disabled unmap.
> 
> Thoughts?

Looks reasonable. I'll update the dw_dmac driver and post a new version
in a few moments.

Haavard

^ permalink raw reply

* Re: [PATCH v4 0/6] dmaengine/mmc: DMA slave interface and two new drivers
From: Dan Williams @ 2008-07-04  1:06 UTC (permalink / raw)
  To: Haavard Skinnemoen
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	shannon.nelson, David Brownell
In-Reply-To: <1214486603-23655-1-git-send-email-haavard.skinnemoen@atmel.com>

On Thu, Jun 26, 2008 at 6:23 AM, Haavard Skinnemoen
<haavard.skinnemoen@atmel.com> wrote:
> First of all, I'm sorry it went so much time between v3 and v4 of this
> patchset. I was hoping to finish this stuff up before all kinds of
> other tasks started demanding my attention, but I didn't, so I had to
> put it on hold for a while. Let's try again...
>
> This patchset extends the DMA engine API to allow drivers to offer DMA
> to and from I/O registers with hardware handshaking, aka slave DMA.
> Such functionality is very common in DMA controllers integrated on SoC
> devices, and it's typically used to do DMA transfers to/from other
> on-SoC peripherals, but it can often do DMA transfers to/from
> externally connected devices as well (e.g. IDE hard drives).
>
> The main differences from v3 of this patchset are:
>  * A DMA descriptor can hold a whole scatterlist. This means that
>    clients using slave DMA can submit large requests in a single call
>    to the driver, and they only need to keep track of a single
>    descriptor.
>  * The dma_slave_descriptor struct is gone since clients no longer
>    need to keep track of multiple descriptors.
>  * The drivers perform better and are more stable.
>
> The dw_dmac driver depends on this patch:
>
>        http://lkml.org/lkml/2008/6/25/148
>
> and the atmel-mci driver depends on this series:
>
>        http://lkml.org/lkml/2008/6/26/158
>
> as well as all preceding patches in this series, of course.
>
> Comments are welcome, as usual! Shortlog and diffstat follow.
>
> Haavard Skinnemoen (6):
>      dmaengine: Add dma_client parameter to device_alloc_chan_resources

Applied.  I fixed it up for fsldma and mv_xor.

>      dmaengine: Add dma_chan_is_in_use() function

I applied the chan->client_count patch that we talked about.

>      dmaengine: Add slave DMA interface

There were some comments to the change log and other fixes, so I'll
wait for v5 of this patch.

>      dmaengine: Make DMA Engine menu visible for AVR32 users

Applied the "remove arch dependency in drivers/dma/Kconfig" instead.

>      dmaengine: Driver for the Synopsys DesignWare DMA controller
>      Atmel MCI: Driver for Atmel on-chip MMC controllers

I will wait for v5 on these as well for the chan->client_count fixups
and a response to the dma_unmap situation.

Thanks,
Dan

^ permalink raw reply

* dmaengine skip unmap (was: Re: [PATCH v4 5/6] dmaengine: Driver for the Synopsys DesignWare DMA controller)
From: Dan Williams @ 2008-07-04  0:40 UTC (permalink / raw)
  To: Haavard Skinnemoen
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	Nelson, Shannon, David Brownell
In-Reply-To: <1214486603-23655-6-git-send-email-haavard.skinnemoen@atmel.com>


On Thu, 2008-06-26 at 06:23 -0700, Haavard Skinnemoen wrote:
> This adds a driver for the Synopsys DesignWare DMA controller (aka
> DMACA on AVR32 systems.) This DMA controller can be found integrated
> on the AT32AP7000 chip and is primarily meant for peripheral DMA
> transfer, but can also be used for memory-to-memory transfers.
> 
> This patch is based on a driver from David Brownell which was based on
> an older version of the DMA Engine framework. It also implements the
> proposed extensions to the DMA Engine API for slave DMA operations.
> 
> The dmatest client shows no problems, but there may still be room for
> improvement performance-wise. DMA slave transfer performance is
> definitely "good enough"; reading 100 MiB from an SD card running at ~20
> MHz yields ~7.2 MiB/s average transfer rate.
> 
> Full documentation for this controller can be found in the Synopsys
> DW AHB DMAC Databook:
> 
> http://www.synopsys.com/designware/docs/iip/DW_ahb_dmac/latest/doc/dw_ahb_dmac_db.pdf
> 
> The controller has lots of implementation options, so it's usually a
> good idea to check the data sheet of the chip it's intergrated on as
> well. The AT32AP7000 data sheet can be found here:
> 
> http://www.atmel.com/dyn/products/datasheets.asp?family_id=682
> 
> Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> 
[..]

> +static void
> +dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
> +{
> +       dma_async_tx_callback           callback;
> +       void                            *param;
> +       struct dma_async_tx_descriptor  *txd = &desc->txd;
> +
> +       dev_vdbg(&dwc->chan.dev, "descriptor %u complete\n", txd->cookie);
> +
> +       dwc->completed = txd->cookie;
> +       callback = txd->callback;
> +       param = txd->callback_param;
> +
> +       dwc_sync_desc_for_cpu(dwc, desc);
> +       list_splice_init(&txd->tx_list, &dwc->free_list);
> +       list_move(&desc->desc_node, &dwc->free_list);
> +
> +       /*
> +        * The API requires that no submissions are done from a
> +        * callback, so we don't need to drop the lock here
> +        */
> +       if (callback)
> +               callback(param);
> +}
> +

The one thing that stands out is that this driver does not unmap the
source or destination buffers (and I now notice that fsldma is not doing
this either, hmm...).  Yes, it is a no-op on avr32, for now, but the
dma-mapping-api assumes that dma_map is always paired with dma_unmap.  I
remember we discussed this earlier and that discussion inspired the
patch below.  The end result is that dw_dmac can try to automatically
dma_unmap the buffers unless an intelligent client, like the mmc driver,
has disabled unmap.

Thoughts?

----snip--->
async_tx: add DMA_COMPL_SKIP_{SRC,DEST}_UNMAP flags to control dma unmap

From: Dan Williams <dan.j.williams@intel.com>

In some cases client code may need the dma-driver to skip the unmap of source
and/or destination buffers.  Setting these flags indicates to the driver to
skip the unmap step.  In this regard async_xor is currently broken in that it
allows the destination buffer to be unmapped while an operation is still in
progress, i.e. when the number of sources exceeds the hardware channel's
maximum (fixed in a subsequent patch).

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

 drivers/dma/ioat_dma.c    |   48 ++++++++++++++++++++++-----------------------
 drivers/dma/iop-adma.c    |   17 ++++++++++++----
 include/linux/dmaengine.h |    4 ++++
 3 files changed, 40 insertions(+), 29 deletions(-)


diff --git a/drivers/dma/ioat_dma.c b/drivers/dma/ioat_dma.c
index 318e8a2..1be33ae 100644
--- a/drivers/dma/ioat_dma.c
+++ b/drivers/dma/ioat_dma.c
@@ -756,6 +756,27 @@ static void ioat_dma_cleanup_tasklet(unsigned long data)
 	       chan->reg_base + IOAT_CHANCTRL_OFFSET);
 }
 
+static void
+ioat_dma_unmap(struct ioat_dma_chan *ioat_chan, struct ioat_desc_sw *desc)
+{
+	/*
+	 * yes we are unmapping both _page and _single
+	 * alloc'd regions with unmap_page. Is this
+	 * *really* that bad?
+	 */
+	if (!(desc->async_tx.flags & DMA_COMPL_SKIP_DEST_UNMAP))
+		pci_unmap_page(ioat_chan->device->pdev,
+				pci_unmap_addr(desc, dst),
+				pci_unmap_len(desc, len),
+				PCI_DMA_FROMDEVICE);
+
+	if (!(desc->async_tx.flags & DMA_COMPL_SKIP_SRC_UNMAP))
+		pci_unmap_page(ioat_chan->device->pdev,
+				pci_unmap_addr(desc, src),
+				pci_unmap_len(desc, len),
+				PCI_DMA_TODEVICE);
+}
+
 /**
  * ioat_dma_memcpy_cleanup - cleanup up finished descriptors
  * @chan: ioat channel to be cleaned up
@@ -816,21 +837,7 @@ static void ioat_dma_memcpy_cleanup(struct ioat_dma_chan *ioat_chan)
 			 */
 			if (desc->async_tx.cookie) {
 				cookie = desc->async_tx.cookie;
-
-				/*
-				 * yes we are unmapping both _page and _single
-				 * alloc'd regions with unmap_page. Is this
-				 * *really* that bad?
-				 */
-				pci_unmap_page(ioat_chan->device->pdev,
-						pci_unmap_addr(desc, dst),
-						pci_unmap_len(desc, len),
-						PCI_DMA_FROMDEVICE);
-				pci_unmap_page(ioat_chan->device->pdev,
-						pci_unmap_addr(desc, src),
-						pci_unmap_len(desc, len),
-						PCI_DMA_TODEVICE);
-
+				ioat_dma_unmap(ioat_chan, desc);
 				if (desc->async_tx.callback) {
 					desc->async_tx.callback(desc->async_tx.callback_param);
 					desc->async_tx.callback = NULL;
@@ -889,16 +896,7 @@ static void ioat_dma_memcpy_cleanup(struct ioat_dma_chan *ioat_chan)
 				if (desc->async_tx.cookie) {
 					cookie = desc->async_tx.cookie;
 					desc->async_tx.cookie = 0;
-
-					pci_unmap_page(ioat_chan->device->pdev,
-						      pci_unmap_addr(desc, dst),
-						      pci_unmap_len(desc, len),
-						      PCI_DMA_FROMDEVICE);
-					pci_unmap_page(ioat_chan->device->pdev,
-						      pci_unmap_addr(desc, src),
-						      pci_unmap_len(desc, len),
-						      PCI_DMA_TODEVICE);
-
+					ioat_dma_unmap(ioat_chan, desc);
 					if (desc->async_tx.callback) {
 						desc->async_tx.callback(desc->async_tx.callback_param);
 						desc->async_tx.callback = NULL;
diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
index 0ec0f43..0b2106e 100644
--- a/drivers/dma/iop-adma.c
+++ b/drivers/dma/iop-adma.c
@@ -82,11 +82,20 @@ iop_adma_run_tx_complete_actions(struct iop_adma_desc_slot *desc,
 			struct device *dev =
 				&iop_chan->device->pdev->dev;
 			u32 len = unmap->unmap_len;
-			u32 src_cnt = unmap->unmap_src_cnt;
-			dma_addr_t addr = iop_desc_get_dest_addr(unmap,
-				iop_chan);
+			enum dma_ctrl_flags flags = desc->async_tx.flags;
+			u32 src_cnt;
+			dma_addr_t addr;
+
+			if (!(flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
+				addr = iop_desc_get_dest_addr(unmap, iop_chan);
+				dma_unmap_page(dev, addr, len, DMA_FROM_DEVICE);
+			}
+
+			if (flags & DMA_COMPL_SKIP_SRC_UNMAP)
+				src_cnt = 0;
+			else
+				src_cnt = unmap->unmap_src_cnt;
 
-			dma_unmap_page(dev, addr, len, DMA_FROM_DEVICE);
 			while (src_cnt--) {
 				addr = iop_desc_get_src_addr(unmap,
 							iop_chan,
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index d08a5c5..78da5c5 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -102,10 +102,14 @@ enum dma_transaction_type {
  * @DMA_CTRL_ACK - the descriptor cannot be reused until the client
  * 	acknowledges receipt, i.e. has has a chance to establish any
  * 	dependency chains
+ * @DMA_COMPL_SKIP_SRC_UNMAP - set to disable dma-unmapping the source buffer(s)
+ * @DMA_COMPL_SKIP_DEST_UNMAP - set to disable dma-unmapping the destination(s)
  */
 enum dma_ctrl_flags {
 	DMA_PREP_INTERRUPT = (1 << 0),
 	DMA_CTRL_ACK = (1 << 1),
+	DMA_COMPL_SKIP_SRC_UNMAP = (1 << 2),
+	DMA_COMPL_SKIP_DEST_UNMAP = (1 << 3),
 };
 
 /**



^ permalink raw reply related

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Andi Kleen @ 2008-07-03  9:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, vda.linux, linux-arch, rmk, dhowells, ralf, kernel,
	jwboyer, paulus, dwmw2, torvalds, paul.gortmaker, linux-embedded,
	linux-kernel, tim.bird, schwidefsky, davem
In-Reply-To: <200807030017.53747.arnd@arndb.de>

Arnd Bergmann wrote:
> On Thursday 03 July 2008, Andi Kleen wrote:
>> Same seems to be true for cris btw.
> 
> Cris has seen significant updates in 2.6.25 by its maintainer.

Hmm missed that, sorry. It was just last time I looked cris
didn't seem to have any updates for a long time.

> It's not a very active port, but skipping updates for one kernel
> version 

It was far longer than that.

-Andi

^ permalink raw reply

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Stephen Rothwell @ 2008-07-03  5:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andi Kleen, Andrew Morton, vda.linux, linux-arch, rmk, dhowells,
	ralf, kernel, jwboyer, paulus, dwmw2, torvalds, paul.gortmaker,
	linux-embedded, linux-kernel, tim.bird, schwidefsky, davem
In-Reply-To: <200807030017.53747.arnd@arndb.de>

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Thu, 3 Jul 2008 00:17:52 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thursday 03 July 2008, Andi Kleen wrote:
> > Same seems to be true for cris btw.
> 
> Cris has seen significant updates in 2.6.25 by its maintainer.
> It's not a very active port, but skipping updates for one kernel
> version is on a completely different scale from doing nothing
> at all for over three years as in the v850 case.

Also, cris has a tree in linux-next now, so there will be updates in 2.6.27.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Arnd Bergmann @ 2008-07-02 22:17 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Andrew Morton, vda.linux, linux-arch, rmk, dhowells, ralf, kernel,
	jwboyer, paulus, dwmw2, torvalds, paul.gortmaker, linux-embedded,
	linux-kernel, tim.bird, schwidefsky, davem
In-Reply-To: <486BFC21.1040001@firstfloor.org>

On Thursday 03 July 2008, Andi Kleen wrote:
> Same seems to be true for cris btw.

Cris has seen significant updates in 2.6.25 by its maintainer.
It's not a very active port, but skipping updates for one kernel
version is on a completely different scale from doing nothing
at all for over three years as in the v850 case.

I don't currently see any architecture (other than v850) in a
state that justifies removing it entirely.

	Arnd <><

^ permalink raw reply

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Andi Kleen @ 2008-07-02 22:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, vda.linux, linux-arch, rmk, dhowells, ralf, kernel,
	jwboyer, paulus, dwmw2, torvalds, paul.gortmaker, linux-embedded,
	linux-kernel, tim.bird, schwidefsky, davem
In-Reply-To: <20080702145859.a9af30ea.akpm@linux-foundation.org>

Andrew Morton wrote:
> On Wed, 2 Jul 2008 23:45:38 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
>> On Wednesday 02 July 2008, Denys Vlasenko wrote:
>>> This patch fixes v850 architecture.
>> For all I know, v850 has been broken and unmaintained for a few years now,
>> didn't someone have a patch to remove it entirely?
>>
> 
> yup, it's queued for 2.6.27-rc1.

Same seems to be true for cris btw.

-Andi




^ permalink raw reply

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Andrew Morton @ 2008-07-02 21:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: vda.linux, linux-arch, rmk, dhowells, ralf, kernel, jwboyer,
	paulus, dwmw2, andi, torvalds, paul.gortmaker, linux-embedded,
	linux-kernel, tim.bird, schwidefsky, davem
In-Reply-To: <200807022345.40184.arnd@arndb.de>

On Wed, 2 Jul 2008 23:45:38 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wednesday 02 July 2008, Denys Vlasenko wrote:
> > This patch fixes v850 architecture.
> 
> For all I know, v850 has been broken and unmaintained for a few years now,
> didn't someone have a patch to remove it entirely?
> 

yup, it's queued for 2.6.27-rc1.

^ permalink raw reply

* Re: [PATCH 21/23] make section names compatible with -ffunction-sections -fdata-sections: v850
From: Arnd Bergmann @ 2008-07-02 21:45 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: linux-arch, Russell King, David Howells, Ralf Baechle,
	Lennert Buytenhek, Josh Boyer, Paul Mackerras, David Woodhouse,
	Andi Kleen, torvalds, akpm, Paul Gortmaker, linux-embedded,
	linux-kernel, Tim Bird, Martin Schwidefsky, Dave Miller
In-Reply-To: <200807020242.21960.vda.linux@googlemail.com>

On Wednesday 02 July 2008, Denys Vlasenko wrote:
> This patch fixes v850 architecture.

For all I know, v850 has been broken and unmaintained for a few years now,
didn't someone have a patch to remove it entirely?

	Arnd <><

^ permalink raw reply

* Re: [PATCH 22/23] make section names compatible with -ffunction-sections -fdata-sections: x86
From: Denys Vlasenko @ 2008-07-02 18:58 UTC (permalink / raw)
  To: linux-arch
  Cc: Russell King, David Howells, Ralf Baechle, Lennert Buytenhek,
	Josh Boyer, Paul Mackerras, David Woodhouse, Andi Kleen, torvalds,
	akpm, Paul Gortmaker, linux-embedded, linux-kernel, Tim Bird,
	Martin Schwidefsky, Dave Miller
In-Reply-To: <200807020242.42414.vda.linux@googlemail.com>

On Wednesday 02 July 2008 02:42, Denys Vlasenko wrote:
> The purpose of this patch is to make kernel buildable
> with "gcc -ffunction-sections -fdata-sections".
> This patch fixes x86 architecture.

Update for x86 arch part. Testing revealed a latent buglet.
arch/x86/boot/compressed/head_64.S did not have "ax",@progbits
and we were only saved by ld being telepathic
(it seems to infer that .text.XXX is code even if
input section attributes are wrong).
head_32.S was ok.
 
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda


--- 0.org/arch/x86/boot/compressed/head_32.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/boot/compressed/head_32.S	Wed Jul  2 00:44:22 2008
@@ -29,7 +29,7 @@
 #include <asm/boot.h>
 #include <asm/asm-offsets.h>
 
-.section ".text.head","ax",@progbits
+.section ".head.text","ax",@progbits
 	.globl startup_32
 
 startup_32:
--- 0.org/arch/x86/boot/compressed/head_64.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/boot/compressed/head_64.S	Wed Jul  2 20:14:26 2008
@@ -32,7 +32,7 @@
 #include <asm/msr.h>
 #include <asm/asm-offsets.h>
 
-.section ".text.head"
+.section ".head.text","ax",@progbits
 	.code32
 	.globl startup_32
 
--- 0.org/arch/x86/boot/compressed/vmlinux.scr	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/boot/compressed/vmlinux.scr	Wed Jul  2 20:10:42 2008
@@ -1,6 +1,6 @@
 SECTIONS
 {
-  .rodata.compressed : {
+  .compressed.rodata : {
 	input_len = .;
 	LONG(input_data_end - input_data) input_data = .;
 	*(.data)
--- 0.org/arch/x86/boot/compressed/vmlinux_32.lds	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/boot/compressed/vmlinux_32.lds	Wed Jul  2 20:33:18 2008
@@ -3,27 +3,27 @@
 ENTRY(startup_32)
 SECTIONS
 {
-	/* Be careful parts of head_32.S assume startup_32 is at
+	/* Be careful, parts of head_32.S assume startup_32 is at
 	 * address 0.
 	 */
 	. = 0;
-	.text.head : {
+	.head.text : {
 		_head = . ;
-		*(.text.head)
+		*(.head.text)
 		_ehead = . ;
 	}
-	.rodata.compressed : {
-		*(.rodata.compressed)
+	.compressed.rodata : {
+		*(.compressed.rodata)
 	}
 	.text :	{
-		_text = .; 	/* Text */
+		_text = .;
 		*(.text)
 		*(.text.*)
 		_etext = . ;
 	}
 	.rodata : {
 		_rodata = . ;
-		*(.rodata)	 /* read-only data */
+		*(.rodata)
 		*(.rodata.*)
 		_erodata = . ;
 	}
@@ -40,4 +40,6 @@
 		*(COMMON)
 		_end = . ;
 	}
+	/* Be bold, and discard everything not explicitly mentioned */
+	/DISCARD/ : { *(*) }
 }
--- 0.org/arch/x86/boot/compressed/vmlinux_64.lds	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/boot/compressed/vmlinux_64.lds	Wed Jul  2 20:32:30 2008
@@ -3,27 +3,27 @@
 ENTRY(startup_64)
 SECTIONS
 {
-	/* Be careful parts of head_64.S assume startup_32 is at
+	/* Be careful, parts of head_64.S assume startup_32 is at
 	 * address 0.
 	 */
 	. = 0;
-	.text.head : {
+	.head.text : {
 		_head = . ;
-		*(.text.head)
+		*(.head.text)
 		_ehead = . ;
 	}
-	.rodata.compressed : {
-		*(.rodata.compressed)
+	.compressed.rodata : {
+		*(.compressed.rodata)
 	}
 	.text :	{
-		_text = .; 	/* Text */
+		_text = .;
 		*(.text)
 		*(.text.*)
 		_etext = . ;
 	}
 	.rodata : {
 		_rodata = . ;
-		*(.rodata)	 /* read-only data */
+		*(.rodata)
 		*(.rodata.*)
 		_erodata = . ;
 	}
@@ -45,4 +45,6 @@
 		. = . + 4096 * 6;
 		_ebss = .;
 	}
+	/* Be bold, and discard everything not explicitly mentioned */
+	/DISCARD/ : { *(*) }
 }
--- 0.org/arch/x86/kernel/acpi/wakeup_32.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/acpi/wakeup_32.S	Wed Jul  2 00:56:50 2008
@@ -1,4 +1,4 @@
-	.section .text.page_aligned
+	.section .page_aligned.text
 #include <linux/linkage.h>
 #include <asm/segment.h>
 #include <asm/page.h>
--- 0.org/arch/x86/kernel/head_32.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/head_32.S	Wed Jul  2 00:47:00 2008
@@ -81,7 +81,7 @@
  * any particular GDT layout, because we load our own as soon as we
  * can.
  */
-.section .text.head,"ax",@progbits
+.section .head.text,"ax",@progbits
 ENTRY(startup_32)
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 		us to not reload segments */
@@ -602,7 +602,7 @@
 /*
  * BSS section
  */
-.section ".bss.page_aligned","wa"
+.section ".bss.k.page_aligned","wa"
 	.align PAGE_SIZE_asm
 #ifdef CONFIG_X86_PAE
 swapper_pg_pmd:
@@ -619,7 +619,7 @@
  * This starts the data section.
  */
 #ifdef CONFIG_X86_PAE
-.section ".data.page_aligned","wa"
+.section ".page_aligned.data","wa"
 	/* Page-aligned for the benefit of paravirt? */
 	.align PAGE_SIZE_asm
 ENTRY(swapper_pg_dir)
--- 0.org/arch/x86/kernel/head_64.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/head_64.S	Wed Jul  2 00:47:00 2008
@@ -32,7 +32,7 @@
  */
 
 	.text
-	.section .text.head
+	.section .head.text
 	.code64
 	.globl startup_64
 startup_64:
@@ -416,7 +416,7 @@
  * Also sysret mandates a special GDT layout 
  */
 		 		
-	.section .data.page_aligned, "aw"
+	.section .page_aligned.data, "aw"
 	.align PAGE_SIZE
 
 /* The TLS descriptors are currently at a different place compared to i386.
@@ -448,7 +448,7 @@
 ENTRY(idt_table)
 	.skip 256 * 16
 
-	.section .bss.page_aligned, "aw", @nobits
+	.section .bss.k.page_aligned, "aw", @nobits
 	.align PAGE_SIZE
 ENTRY(empty_zero_page)
 	.skip PAGE_SIZE
--- 0.org/arch/x86/kernel/init_task.c	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/init_task.c	Wed Jul  2 00:45:57 2008
@@ -24,7 +24,7 @@
  * "init_task" linker map entry..
  */
 union thread_union init_thread_union
-	__attribute__((__section__(".data.init_task"))) =
+	__attribute__((__section__(".init_task.data"))) =
 		{ INIT_THREAD_INFO(init_task) };
 
 /*
@@ -38,7 +38,7 @@
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
  * no more per-task TSS's. The TSS size is kept cacheline-aligned
- * so they are allowed to end up in the .data.cacheline_aligned
+ * so they are allowed to end up in the .cacheline_aligned.data
  * section. Since TSS's are completely CPU-local, we want them
  * on exact cacheline boundaries, to eliminate cacheline ping-pong.
  */
--- 0.org/arch/x86/kernel/irq_32.c	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/irq_32.c	Wed Jul  2 00:47:00 2008
@@ -148,10 +148,10 @@
 #ifdef CONFIG_4KSTACKS
 
 static char softirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__section__(".bss.page_aligned")));
+		__attribute__((__section__(".bss.k.page_aligned")));
 
 static char hardirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__section__(".bss.page_aligned")));
+		__attribute__((__section__(".bss.k.page_aligned")));
 
 /*
  * allocate per-cpu stacks for hardirq and for softirq processing
--- 0.org/arch/x86/kernel/setup64.c	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/setup64.c	Wed Jul  2 00:47:00 2008
@@ -40,7 +40,7 @@
 
 struct desc_ptr idt_descr = { 256 * 16 - 1, (unsigned long) idt_table };
 
-char boot_cpu_stack[IRQSTACKSIZE] __attribute__((section(".bss.page_aligned")));
+char boot_cpu_stack[IRQSTACKSIZE] __attribute__((section(".bss.k.page_aligned")));
 
 unsigned long __supported_pte_mask __read_mostly = ~0UL;
 EXPORT_SYMBOL_GPL(__supported_pte_mask);
@@ -121,7 +121,7 @@
 } 
 
 char boot_exception_stacks[(N_EXCEPTION_STACKS - 1) * EXCEPTION_STKSZ + DEBUG_STKSZ]
-__attribute__((section(".bss.page_aligned")));
+__attribute__((section(".bss.k.page_aligned")));
 
 extern asmlinkage void ignore_sysret(void);
 
--- 0.org/arch/x86/kernel/traps_32.c	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/traps_32.c	Wed Jul  2 00:46:32 2008
@@ -76,7 +76,7 @@
  * for this.
  */
 gate_desc idt_table[256]
-	__attribute__((__section__(".data.idt"))) = { { { { 0, 0 } } }, };
+	__attribute__((__section__(".idt.data"))) = { { { { 0, 0 } } }, };
 
 asmlinkage void divide_error(void);
 asmlinkage void debug(void);
--- 0.org/arch/x86/kernel/vmlinux_32.lds.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/vmlinux_32.lds.S	Wed Jul  2 00:56:50 2008
@@ -31,15 +31,15 @@
   . = LOAD_OFFSET + LOAD_PHYSICAL_ADDR;
   phys_startup_32 = startup_32 - LOAD_OFFSET;
 
-  .text.head : AT(ADDR(.text.head) - LOAD_OFFSET) {
+  .head.text : AT(ADDR(.head.text) - LOAD_OFFSET) {
   	_text = .;			/* Text and read-only data */
-	*(.text.head)
+	*(.head.text)
   } :text = 0x9090
 
   /* read-only */
   .text : AT(ADDR(.text) - LOAD_OFFSET) {
 	. = ALIGN(PAGE_SIZE); /* not really needed, already page aligned */
-	*(.text.page_aligned)
+	*(.page_aligned.text)
 	TEXT_TEXT
 	SCHED_TEXT
 	LOCK_TEXT
@@ -79,32 +79,32 @@
   . = ALIGN(PAGE_SIZE);
   .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) {
   	__nosave_begin = .;
-	*(.data.nosave)
+	*(.nosave.data)
   	. = ALIGN(PAGE_SIZE);
   	__nosave_end = .;
   }
 
   . = ALIGN(PAGE_SIZE);
-  .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
-	*(.data.page_aligned)
-	*(.data.idt)
+  .page_aligned.data : AT(ADDR(.page_aligned.data) - LOAD_OFFSET) {
+	*(.page_aligned.data)
+	*(.idt.data)
   }
 
   . = ALIGN(32);
-  .data.cacheline_aligned : AT(ADDR(.data.cacheline_aligned) - LOAD_OFFSET) {
-	*(.data.cacheline_aligned)
+  .cacheline_aligned.data : AT(ADDR(.cacheline_aligned.data) - LOAD_OFFSET) {
+	*(.cacheline_aligned.data)
   }
 
   /* rarely changed data like cpu maps */
   . = ALIGN(32);
-  .data.read_mostly : AT(ADDR(.data.read_mostly) - LOAD_OFFSET) {
-	*(.data.read_mostly)
+  .read_mostly.data : AT(ADDR(.read_mostly.data) - LOAD_OFFSET) {
+	*(.read_mostly.data)
 	_edata = .;		/* End of data section */
   }
 
   . = ALIGN(THREAD_SIZE);	/* init_task */
-  .data.init_task : AT(ADDR(.data.init_task) - LOAD_OFFSET) {
-	*(.data.init_task)
+  .init_task.data : AT(ADDR(.init_task.data) - LOAD_OFFSET) {
+	*(.init_task.data)
   }
 
   /* might get freed after init */
@@ -187,10 +187,10 @@
   }
 #endif
   . = ALIGN(PAGE_SIZE);
-  .data.percpu  : AT(ADDR(.data.percpu) - LOAD_OFFSET) {
+  .percpu.data  : AT(ADDR(.percpu.data) - LOAD_OFFSET) {
 	__per_cpu_start = .;
-	*(.data.percpu)
-	*(.data.percpu.shared_aligned)
+	*(.percpu.data)
+	*(.percpu.shared_aligned.data)
 	__per_cpu_end = .;
   }
   . = ALIGN(PAGE_SIZE);
@@ -199,7 +199,7 @@
   .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
 	__init_end = .;
 	__bss_start = .;		/* BSS */
-	*(.bss.page_aligned)
+	*(.bss.k.page_aligned)
 	*(.bss)
 	. = ALIGN(4);
 	__bss_stop = .;
--- 0.org/arch/x86/kernel/vmlinux_64.lds.S	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/kernel/vmlinux_64.lds.S	Wed Jul  2 00:47:00 2008
@@ -28,7 +28,7 @@
   _text = .;			/* Text and read-only data */
   .text :  AT(ADDR(.text) - LOAD_OFFSET) {
 	/* First the code that has to be first for bootstrapping */
-	*(.text.head)
+	*(.head.text)
 	_stext = .;
 	/* Then the rest */
 	TEXT_TEXT
@@ -71,17 +71,17 @@
 
   . = ALIGN(PAGE_SIZE);
   . = ALIGN(CONFIG_X86_L1_CACHE_BYTES);
-  .data.cacheline_aligned : AT(ADDR(.data.cacheline_aligned) - LOAD_OFFSET) {
-	*(.data.cacheline_aligned)
+  .cacheline_aligned.data : AT(ADDR(.cacheline_aligned.data) - LOAD_OFFSET) {
+	*(.cacheline_aligned.data)
   }
   . = ALIGN(CONFIG_X86_INTERNODE_CACHE_BYTES);
-  .data.read_mostly : AT(ADDR(.data.read_mostly) - LOAD_OFFSET) {
-  	*(.data.read_mostly)
+  .read_mostly.data : AT(ADDR(.read_mostly.data) - LOAD_OFFSET) {
+  	*(.read_mostly.data)
   }
 
 #define VSYSCALL_ADDR (-10*1024*1024)
-#define VSYSCALL_PHYS_ADDR ((LOADADDR(.data.read_mostly) + SIZEOF(.data.read_mostly) + 4095) & ~(4095))
-#define VSYSCALL_VIRT_ADDR ((ADDR(.data.read_mostly) + SIZEOF(.data.read_mostly) + 4095) & ~(4095))
+#define VSYSCALL_PHYS_ADDR ((LOADADDR(.read_mostly.data) + SIZEOF(.read_mostly.data) + 4095) & ~(4095))
+#define VSYSCALL_VIRT_ADDR ((ADDR(.read_mostly.data) + SIZEOF(.read_mostly.data) + 4095) & ~(4095))
 
 #define VLOAD_OFFSET (VSYSCALL_ADDR - VSYSCALL_PHYS_ADDR)
 #define VLOAD(x) (ADDR(x) - VLOAD_OFFSET)
@@ -130,13 +130,13 @@
 #undef VVIRT
 
   . = ALIGN(THREAD_SIZE);	/* init_task */
-  .data.init_task : AT(ADDR(.data.init_task) - LOAD_OFFSET) {
-	*(.data.init_task)
+  .init_task.data : AT(ADDR(.init_task.data) - LOAD_OFFSET) {
+	*(.init_task.data)
   }:data.init
 
   . = ALIGN(PAGE_SIZE);
-  .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
-	*(.data.page_aligned)
+  .page_aligned.data : AT(ADDR(.page_aligned.data) - LOAD_OFFSET) {
+	*(.page_aligned.data)
   }
 
   /* might get freed after init */
@@ -223,13 +223,13 @@
 
   . = ALIGN(PAGE_SIZE);
   __nosave_begin = .;
-  .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) { *(.data.nosave) }
+  .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) { *(.nosave.data) }
   . = ALIGN(PAGE_SIZE);
   __nosave_end = .;
 
   __bss_start = .;		/* BSS */
   .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
-	*(.bss.page_aligned)
+	*(.bss.k.page_aligned)
 	*(.bss)
 	}
   __bss_stop = .;
--- 0.org/arch/x86/mm/ioremap.c	Wed Jul  2 00:40:42 2008
+++ 1.fixname/arch/x86/mm/ioremap.c	Wed Jul  2 00:47:00 2008
@@ -395,7 +395,7 @@
 
 static __initdata int after_paging_init;
 static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)]
-		__section(.bss.page_aligned);
+		__section(.bss.k.page_aligned);
 
 static inline pmd_t * __init early_ioremap_pmd(unsigned long addr)
 {
--- 0.org/include/asm-x86/cache.h	Wed Jul  2 00:40:51 2008
+++ 1.fixname/include/asm-x86/cache.h	Wed Jul  2 00:46:09 2008
@@ -5,7 +5,7 @@
 #define L1_CACHE_SHIFT	(CONFIG_X86_L1_CACHE_SHIFT)
 #define L1_CACHE_BYTES	(1 << L1_CACHE_SHIFT)
 
-#define __read_mostly __attribute__((__section__(".data.read_mostly")))
+#define __read_mostly __attribute__((__section__(".read_mostly.data")))
 
 #ifdef CONFIG_X86_VSMP
 /* vSMP Internode cacheline shift */
@@ -13,7 +13,7 @@
 #ifdef CONFIG_SMP
 #define __cacheline_aligned_in_smp					\
 	__attribute__((__aligned__(1 << (INTERNODE_CACHE_SHIFT))))	\
-	__attribute__((__section__(".data.page_aligned")))
+	__attribute__((__section__(".page_aligned.data")))
 #endif
 #endif
 

^ permalink raw reply

* Re: [PATCH 14/23] make section names compatible with -ffunction-sections -fdata-sections: parisc
From: Adrian Bunk @ 2008-07-02 15:06 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Denys Vlasenko, James Bottomley, linux-arch, Russell King,
	David Howells, Ralf Baechle, Lennert Buytenhek, Josh Boyer,
	Paul Mackerras, Andi Kleen, torvalds, akpm, Paul Gortmaker,
	linux-embedded, linux-kernel, Tim Bird, Martin Schwidefsky,
	Dave Miller
In-Reply-To: <1215010662.10393.459.camel@pmac.infradead.org>

On Wed, Jul 02, 2008 at 03:57:42PM +0100, David Woodhouse wrote:
> On Wed, 2008-07-02 at 17:55 +0300, Adrian Bunk wrote:
> > On Wed, Jul 02, 2008 at 09:41:47AM +0100, David Woodhouse wrote:
> > > On Wed, 2008-07-02 at 02:00 +0200, Denys Vlasenko wrote:
> > > > On Wednesday 02 July 2008 01:41, James Bottomley wrote:
> > > > > On Wed, 2008-07-02 at 02:39 +0200, Denys Vlasenko wrote:
> > > > > > The purpose of this patch is to make kernel buildable
> > > > > > with "gcc -ffunction-sections -fdata-sections".
> > > > > > This patch fixes parisc architecture.
> > > > > > 
> > > > > > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
> > > > > 
> > > > > Um ... if you look at the Makefile you'll see we already build parisc
> > > > > with -ffunction-sections; we have to: our relative jumps are too small
> > > > > to guarantee finding the stubs in large files.
> > > > > 
> > > > > Since our text is -ffunction-sections compatible already, I question the
> > > > > need for transformations like this:
> > > 
> > > We've been building FR-V kernels with --gc-sections for a long time,
> > > too.
> > >...
> > 
> > Is there any specific reason why it's not done in the upstream kernel?
> 
> Isn't it? I thought it was.
>...

It is in the Makefile, but only enabled through a nonexisting 
CONFIG_GC_SECTIONS option.

> dwmw2

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply

* Re: [PATCH 14/23] make section names compatible with -ffunction-sections -fdata-sections: parisc
From: David Woodhouse @ 2008-07-02 14:57 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Denys Vlasenko, James Bottomley, linux-arch, Russell King,
	David Howells, Ralf Baechle, Lennert Buytenhek, Josh Boyer,
	Paul Mackerras, Andi Kleen, torvalds, akpm, Paul Gortmaker,
	linux-embedded, linux-kernel, Tim Bird, Martin Schwidefsky,
	Dave Miller
In-Reply-To: <20080702145514.GC4196@cs181140183.pp.htv.fi>

On Wed, 2008-07-02 at 17:55 +0300, Adrian Bunk wrote:
> On Wed, Jul 02, 2008 at 09:41:47AM +0100, David Woodhouse wrote:
> > On Wed, 2008-07-02 at 02:00 +0200, Denys Vlasenko wrote:
> > > On Wednesday 02 July 2008 01:41, James Bottomley wrote:
> > > > On Wed, 2008-07-02 at 02:39 +0200, Denys Vlasenko wrote:
> > > > > The purpose of this patch is to make kernel buildable
> > > > > with "gcc -ffunction-sections -fdata-sections".
> > > > > This patch fixes parisc architecture.
> > > > > 
> > > > > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
> > > > 
> > > > Um ... if you look at the Makefile you'll see we already build parisc
> > > > with -ffunction-sections; we have to: our relative jumps are too small
> > > > to guarantee finding the stubs in large files.
> > > > 
> > > > Since our text is -ffunction-sections compatible already, I question the
> > > > need for transformations like this:
> > 
> > We've been building FR-V kernels with --gc-sections for a long time,
> > too.
> >...
> 
> Is there any specific reason why it's not done in the upstream kernel?

Isn't it? I thought it was.

Or maybe we were only doing that before we added MMU support. I remember
the exception tables complicate matters a little.

-- 
dwmw2

^ permalink raw reply

* Re: [PATCH 14/23] make section names compatible with -ffunction-sections -fdata-sections: parisc
From: Adrian Bunk @ 2008-07-02 14:55 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Denys Vlasenko, James Bottomley, linux-arch, Russell King,
	David Howells, Ralf Baechle, Lennert Buytenhek, Josh Boyer,
	Paul Mackerras, Andi Kleen, torvalds, akpm, Paul Gortmaker,
	linux-embedded, linux-kernel, Tim Bird, Martin Schwidefsky,
	Dave Miller
In-Reply-To: <1214988107.10393.444.camel@pmac.infradead.org>

On Wed, Jul 02, 2008 at 09:41:47AM +0100, David Woodhouse wrote:
> On Wed, 2008-07-02 at 02:00 +0200, Denys Vlasenko wrote:
> > On Wednesday 02 July 2008 01:41, James Bottomley wrote:
> > > On Wed, 2008-07-02 at 02:39 +0200, Denys Vlasenko wrote:
> > > > The purpose of this patch is to make kernel buildable
> > > > with "gcc -ffunction-sections -fdata-sections".
> > > > This patch fixes parisc architecture.
> > > > 
> > > > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
> > > 
> > > Um ... if you look at the Makefile you'll see we already build parisc
> > > with -ffunction-sections; we have to: our relative jumps are too small
> > > to guarantee finding the stubs in large files.
> > > 
> > > Since our text is -ffunction-sections compatible already, I question the
> > > need for transformations like this:
> 
> We've been building FR-V kernels with --gc-sections for a long time,
> too.
>...

Is there any specific reason why it's not done in the upstream kernel?

> dwmw2

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply

* Re: [PATCH 16/23] make section names compatible with -ffunction-sections -fdata-sections: ppc
From: Josh Boyer @ 2008-07-02 11:54 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: linux-arch, Russell King, David Howells, Ralf Baechle,
	Lennert Buytenhek, Paul Mackerras, David Woodhouse, Andi Kleen,
	torvalds, akpm, Paul Gortmaker, linux-embedded, linux-kernel,
	Tim Bird, Martin Schwidefsky, Dave Miller
In-Reply-To: <200807020240.03443.vda.linux@googlemail.com>

On Wed, 2 Jul 2008 02:40:03 +0200
Denys Vlasenko <vda.linux@googlemail.com> wrote:

> The purpose of this patch is to make kernel buildable
> with "gcc -ffunction-sections -fdata-sections".
> This patch fixes ppc architecture.
> 
> Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

You can drop this one entirely.  arch/ppc won't exist in 2.6.27.  It
has been removed in the powerpc tree already.

josh

^ permalink raw reply

* Re: [PATCH 14/23] make section names compatible with -ffunction-sections -fdata-sections: parisc
From: David Woodhouse @ 2008-07-02  8:41 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: James Bottomley, linux-arch, Russell King, David Howells,
	Ralf Baechle, Lennert Buytenhek, Josh Boyer, Paul Mackerras,
	Andi Kleen, torvalds, akpm, Paul Gortmaker, linux-embedded,
	linux-kernel, Tim Bird, Martin Schwidefsky, Dave Miller
In-Reply-To: <200807020200.49518.vda.linux@googlemail.com>

On Wed, 2008-07-02 at 02:00 +0200, Denys Vlasenko wrote:
> On Wednesday 02 July 2008 01:41, James Bottomley wrote:
> > On Wed, 2008-07-02 at 02:39 +0200, Denys Vlasenko wrote:
> > > The purpose of this patch is to make kernel buildable
> > > with "gcc -ffunction-sections -fdata-sections".
> > > This patch fixes parisc architecture.
> > > 
> > > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
> > 
> > Um ... if you look at the Makefile you'll see we already build parisc
> > with -ffunction-sections; we have to: our relative jumps are too small
> > to guarantee finding the stubs in large files.
> > 
> > Since our text is -ffunction-sections compatible already, I question the
> > need for transformations like this:

We've been building FR-V kernels with --gc-sections for a long time,
too.

> In order to handle these situations uniformly, in these patches
> I decided to _never_ use .text.XXXX names for sections,
> effectively leaving them "reserved for gcc's use".

It makes a certain amount of sense to do this uniformly for all
architectures -- leaving .text.* and .data.* for GCC, and using
something else whenever we manually name sections. 

Denys' patch for parisc would need a little more thought, but it's
probably worth it to be consistent. Especially if we can move more stuff
out of athe arch-specific linker scripts and into
<asm-generic/vmlinux.lds.h>

-- 
dwmw2

^ permalink raw reply

* Re: [PATCH v4 2/6] dmaengine: Add dma_chan_is_in_use() function
From: Haavard Skinnemoen @ 2008-07-02  7:59 UTC (permalink / raw)
  To: Dan Williams
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	shannon.nelson, David Brownell
In-Reply-To: <e9c3a7c20807011900x7b2ac3dx7536e0450202e3c0@mail.gmail.com>

"Dan Williams" <dan.j.williams@intel.com> wrote:
> Actually we will probably need something like the following.
> ->client_count is protected by the dma_list_mutex.  
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 99c22b4..10de69e 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -183,9 +183,10 @@ static void dma_client_chan_alloc(struct
> dma_client *client)
>  				/* we are done once this client rejects
>  				 * an available resource
>  				 */
> -				if (ack == DMA_ACK)
> +				if (ack == DMA_ACK) {
>  					dma_chan_get(chan);
> -				else if (ack == DMA_NAK)
> +					chan->client_count++;
> +				} else if (ack == DMA_NAK)
>  					return;
>  			}

This looks good to me. I can use client_count to determine if dwc->dws
is actually valid so that channels that were initially allocated for a
slave but NAK'ed or DUP'ed can be reclaimed for other purposes.

It still doesn't solve the issue with memory wastage, but we probably
shouldn't expect to keep a lot of unused channels around anyway.

Thanks!

Haavard

^ permalink raw reply

* Re: [PATCH v4 2/6] dmaengine: Add dma_chan_is_in_use() function
From: Haavard Skinnemoen @ 2008-07-02  7:56 UTC (permalink / raw)
  To: Dan Williams
  Cc: Pierre Ossman, linux-kernel, linux-embedded, kernel,
	shannon.nelson, David Brownell
In-Reply-To: <e9c3a7c20807011831n74eb69b8r3095b62515fe5c9@mail.gmail.com>

"Dan Williams" <dan.j.williams@intel.com> wrote:
> On Thu, Jun 26, 2008 at 6:23 AM, Haavard Skinnemoen
> <haavard.skinnemoen@atmel.com> wrote:
> > This moves the code checking if a DMA channel is in use from
> > show_in_use() into an inline helper function, dma_is_in_use(). DMA
> > controllers can use this in order to give clients exclusive access to
> > channels (usually necessary when setting up slave DMA.)
> >
> > I have to admit that I don't really understand the channel refcounting
> > logic at all... dma_chan_get() simply increments a per-cpu value. How
> > can we be sure that whatever CPU calls dma_chan_is_in_use() sees the
> > same value?
> 
> As Chris noted in the comments at the top of dmaengine.c this is an
> implementation Rusty's 'bigref'.  It seeks to avoid the
> cache-line-bouncing overhead of maintaining a single global refcount
> in hot paths like tcp_v{4,6}_rcv().  When the channel is being
> removed, a rare event, we transition to the accurate, yet slow, global
> method.

Ok, I was sort of wondering what happens if you call dma_chan_get() on
one cpu and dma_chan_put() on a different cpu later on. But it looks
like when it really matters, the sum across all cpus is used, so the end
result will be correct.

> Your observation is correct, dma_chan_is_in_use() may lie in the case
> when the current cpu is not using the channel.  For this particular
> test I think you can look to see if this channel's resources are
> already allocated.  If they are then some other client got a hold of
> this channel before the current attempt.  Hmm... that would also
> require that we free the channel's resources in the case where the
> client replies with DMA_NAK, probably something we should do anyways.

Yes, I think that's good thing to do in general. In fact, I think the
dw_dmac driver will waste a channel for each slave because it always
assigns the channel to the client even if the client may NAK or DUP it
later on. I haven't seen this actually happening because I only have
one slave client at the moment.

Another reason to do this is to reclaim the memory used for
descriptors. Currently, a channel that was NAK'ed or DUP'ed will still
have a lot of preallocated descriptors, possibly with client-specific
parameters already set up.

Haavard

^ permalink raw reply

* Re: [PATCH 1/23] make section names compatible with -ffunction-sections -fdata-sections
From: Denys Vlasenko @ 2008-07-02  7:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-arch, Russell King, David Howells, Ralf Baechle,
	Lennert Buytenhek, Josh Boyer, Paul Mackerras, David Woodhouse,
	Andi Kleen, torvalds, Paul Gortmaker, linux-embedded,
	linux-kernel, Tim Bird, Martin Schwidefsky, Dave Miller
In-Reply-To: <20080701213006.5502987a.akpm@linux-foundation.org>

On Wednesday 02 July 2008 06:30, Andrew Morton wrote:
> On Wed, 2 Jul 2008 02:33:48 +0200 Denys Vlasenko <vda.linux@googlemail.com> wrote:
> > I am unsure how to synchronize propagation of these patches
> > across all architectures.
> > 
> > Andrew, how this can be done without causing lots of pain
> > for arch maintainers? Please advise.
> 
> You didn't describe the problem which you're trying to solve, so how
> can I say?

The problem is that with -ffunction-sections -fdata-sections gcc
will create sections like .text.head and .data.nosave
whenever someone will have innocuous code like this:

static void head(...) {...}

or this:

int f(...)
{ 
	static int nosave;
...
}

somewhere in the kernel.

Then kernel linker script will be confused and put these sections
in wrong places.

IOW: names like .text.XXXX and .data.XXX must not be used for "magic"
sections.


> Possibilities are:
> 
> a) the generic bit depends on the arch bits
> 
>    -> No probs.  I can merge the generic bit once all architectures are in.
> 
> b) the arch bits depend on the generic bits
> 
>    -> No probs.  I can merge the generic bit then send all the arch bits.
> 
> c) they each depend on each other
> 
>    -> No probs.  We go round gaththering acks, slam it all into
>       a single patch then in it goes.  2.6.28, presumably.

It's definitely (c). Changes in, say, include/linux/init.h:

-#define __nosavedata __section(.data.nosave)
+#define __nosavedata __section(.nosave.data)

must be syncronized with, say, arch/arm/kernel/vmlinux.lds.S:

                . = ALIGN(4096);
                __nosave_begin = .;
-               *(.data.nosave)
+               *(.nosave.data)

> > The following patches fix section names, one per architecture.
> > 
> > The patch in _this_ mail fixes generic part.
> 
> (tries to work out what it does)
> 
> oh, it does the above section renaming.  So I guess we're looking at
> scenario c), above?
> 
> "otherwise section placement done by kernel's custom linker scripts
> produces broken vmlinux and vdso images" is an inadequate description. 
> Please describe the problem more completely.  This is important,
> because once we actually find out what the patch is fixing, perhaps
> others will be aware of less intrusive ways of fixing the problem, and
> we end up with a better patch.

See above. Is that explanation ok?

> Please be aware that last time someone tried function-sections, maybe
> five years ago, problems were encountered with linker efficiency
> (possible an O(nsections) or worse algorithm in ld).  Link times went
> up a lot.

Last time is was probably me :) about a year ago I think.
Last link stage takes niticeably more time, but
nothing really awful.

> So it would be good to hunt down some old ld versions and run some
> timings.  A mention of the results in the changelog is appropriate.
> 
> Is there actually a patch anywhere which enables function-sections for
> some architectures?  It would be good to see that (and its associated
> size-reduction results) so we can work out whether all these changes
> are worth pursuing.

Yes, I was posting it twice during last year.
(digging up old emails from "sent" folder...) here is some:

On Friday 07 September 2007 19:30, Denys Vlasenko wrote:
> On Friday 07 September 2007 17:31, Daniel Walker wrote:
> > On Thu, 2007-09-06 at 18:07 +0100, Denys Vlasenko wrote:
> > > A bit extended version:
> > > 
> > > In the process in making it work I saw ~10% vmlinux size reductions
> > > (which basically matches what Marcelo says) when I wasn't retaining
> > > sections needed for EXPORT_SYMBOLs, but module loading didn't work.
> > > 
> > > Thus I fixed that by adding KEEP() directives so that EXPORT_SYMBOLs
> > > are never discarded. This was just one of many fixes until kernel
> > > started to actually boot and work.
> > > 
> > > I did that before I posted patches to lkml.
> > > IOW: posted patches are not broken versus module loading.
> > 
> > Ok, this is more like the explanation I was looking for..
> > 
> > During this thread you seemed to indicate the patches you release
> > reduced the kernel ~10% , but now your saying that was pre-release ,
> > right?
> 
> CONFIG_MODULE=n will save ~10%
> CONFIG_MODULE=y - ~1%
> 
> Exact figure depends on .config (whether you happen to include
> especially "fat" code or not).

--
vda

^ permalink raw reply

* Re: [PATCH 3/23] make section names compatible with -ffunction-sections -fdata-sections: arm
From: Roberto A. Foglietta @ 2008-07-02  6:22 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: linux-arch, Russell King, David Howells, Ralf Baechle,
	Lennert Buytenhek, Josh Boyer, Paul Mackerras, David Woodhouse,
	Andi Kleen, torvalds, akpm, Paul Gortmaker, linux-embedded,
	linux-kernel, Tim Bird, Martin Schwidefsky, Dave Miller
In-Reply-To: <200807020234.26755.vda.linux@googlemail.com>

2008/7/2 Denys Vlasenko <vda.linux@googlemail.com>:
> The purpose of this patch is to make kernel buildable
> with "gcc -ffunction-sections -fdata-sections".
> This patch fixes arm architecture.
>

Hi Denys,

 I see your patchset. may I suggest you to take a look to this tools?

 http://www.emn.fr/x-info/coccinelle/

 I think it could simplify this kind of patching for the future.
 I found it reading the kernel janitors m-list and I hope it helps.

 Cheers,
-- 
/roberto

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox