linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver
@ 2014-07-23  8:24 Yuan Yao
  2014-07-23  8:24 ` [PATCH v5 1/2] " Yuan Yao
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Yuan Yao @ 2014-07-23  8:24 UTC (permalink / raw)
  To: wsa, marex
  Cc: mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c


Changed in v5:
- add "*chan_dev = dma->chan_using->device->dev" for reduce the call time.
- add the test logs.

Changed in v4:
- cancelled  "i2c_imx->use_dma".
- changed "Dma" to "DMA".
- add Timeout handling  for Transfer complete.

Changed in v3:
- fix a bug when request the dma faild.
- some minor fixes for coding style.
- other minor fixes.

Changed in v2:
- remove has_dma_support property
- unify i2c_imx_dma_rx and i2c_imx_dma_tx
- unify i2c_imx_dma_read and i2c_imx_pio_read
- unify i2c_imx_dma_write and i2c_imx_pio_write

Added in v1:
- Enable dma if it's support dma and transfer size bigger than the threshold.
- Add device tree bindings for i2c eDMA support.
- Add eDMA support for i2c driver.

The test log:(one EEPROM on I2C2, use i2cdump tools)


U-Boot 2014.01-00634-g22b2f38-dirty (Jul 17 2014 - 17:16:37)

CPU:   Freescale LayerScape SLS1020, Version: 1.0, (0x87080010)
Clock Configuration:
       CPU0(ARMV7):1000 MHz,
       Bus:300  MHz, DDR:800  MHz (1600 MT/s data rate), Reset Configuration Word (RCW):
       00000000: 0608000a 00000000 00000000 00000000
       00000010: 20000000 00407900 e0025a00 21046000
       00000020: 00000000 00000000 00000000 00038000
       00000030: 00000000 881b7540 00000000 00000000
Board: LS1021ATWR
CPLD:  V1.1
PCBA:  V2.0
VBank: 1
I2C:   ready
DRAM:  PGTABLE_SIZE 4000, gd->relocaddr c0000000
gd->relocaddr bfffc000
gd->relocaddr bfff0000
TLB table from bfff0000 to bfff4000
1 GiB
Using SERDES1 Protocol: 32 (0x20)
Flash: 128 MiB
MMC:   FSL_SDHC: 0
EEPROM: ret 1 0
ret 2 0
ID: NXID v16777216
SN:
Errata:
Build date: 2014/07/17 12:12:12
Eth0: 00:04:9f:ef:00:00
Eth1: 00:04:9f:ef:01:01
Eth2: 00:04:9f:ef:02:02
Eth3: 00:04:9f:ef:03:03
CRC: bf83b86a
NXID v16777216
In:    serial
Out:   serial
Err:   serial
Net:   eTSEC1 is in sgmii mode.
eTSEC2 is in sgmii mode.
eTSEC1 [PRIME], eTSEC2, eTSEC3
=> run yyboot
Speed: 1000, full duplex
BOOTP broadcast 1
BOOTP broadcast 2
*** Unhandled DHCP Option in OFFER/ACK: 44
*** Unhandled DHCP Option in OFFER/ACK: 46
*** Unhandled DHCP Option in OFFER/ACK: 44
*** Unhandled DHCP Option in OFFER/ACK: 46 DHCP client bound to address 10.193.20.30 Using eTSEC1 device TFTP from server 10.193.20.35; our IP address is 10.193.20.30 Filename 'twr/uImage.ls1'.
Load address: 0x82000000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         ###################################
         2.7 MiB/s
done
Bytes transferred = 6236192 (5f2820 hex)
Speed: 1000, full duplex
Using eTSEC1 device
TFTP from server 10.193.20.35; our IP address is 10.193.20.30 Filename 'twr/ls1021a-twr.dtb'.
Load address: 0x8f000000
Loading: ##
         229.5 KiB/s
done
Bytes transferred = 19995 (4e1b hex)
Speed: 1000, full duplex
Using eTSEC1 device
TFTP from server 10.193.20.35; our IP address is 10.193.20.30 Filename 'twr/Ramdisk.uboot'.
Load address: 0x88000000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #####
         2.9 MiB/s
done
Bytes transferred = 19156457 (1244de9 hex) ## Booting kernel from Legacy Image at 82000000 ...
   Image Name:   Linux Kernel
   Image Type:   ARM Linux Kernel Image (uncompressed)
   Data Size:    6236128 Bytes = 5.9 MiB
   Load Address: 80008000
   Entry Point:  80008000
   Verifying Checksum ... OK
## Loading init Ramdisk from Legacy Image at 88000000 ...
   Image Name:   fsl-image-core-ls1021atwr-201406
   Image Type:   ARM Linux RAMDisk Image (gzip compressed)
   Data Size:    19156393 Bytes = 18.3 MiB
   Load Address: 00000000
   Entry Point:  00000000
   Verifying Checksum ... OK
## Flattened Device Tree blob at 8f000000
   Booting using the fdt blob at 0x8f000000
   Loading Kernel Image ... OK
   Using Device Tree in place at 8f000000, end 8f007e1a

Starting kernel ...

Booting Linux on physical CPU 0xf00
Linux version 3.12.0+ (b46683@rock) (gcc version 4.8.3 20131202 (prerelease) (cr                                         osstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC 2013.11) ) #190 SMP Fri Jul 18                                          14:24:11 CST 2014
CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=30c73c7d
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
Machine: Freescale Layerscape LS1021A, model: LS1021A TWR Board Memory policy: ECC disabled, Data cache writealloc
PERCPU: Embedded 7 pages/cpu @89a56000 s7680 r8192 d12800 u32768 Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 260096 Kernel command line: root=/dev/ram0 rw console=ttyS0,115200 PID hash table entries: 4096 (order: 2, 16384 bytes) Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1014380K/1048576K available (4367K kernel code, 241K rwdata, 1288K rodat                                         a, 191K init, 215K bss, 34196K reserved, 0K highmem)
Virtual kernel memory layout:
    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xc0800000 - 0xff000000   (1000 MB)
    lowmem  : 0x80000000 - 0xc0000000   (1024 MB)
    pkmap   : 0x7fe00000 - 0x80000000   (   2 MB)
    modules : 0x7f000000 - 0x7fe00000   (  14 MB)
      .text : 0x80008000 - 0x8058df74   (5656 kB)
      .init : 0x8058e000 - 0x805bde00   ( 192 kB)
      .data : 0x805be000 - 0x805fa7d8   ( 242 kB)
       .bss : 0x805fa7e0 - 0x806305a8   ( 216 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1 Hierarchical RCU implementation.
        RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
NR_IRQS:16 nr_irqs:16 16
Architected cp15 timer(s) running at 12.50MHz (phys).
Switching to timer-based delay loop
sched_clock: ARM arch timer >56 bits at 12500kHz, resolution 80ns
sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 4294967286ms
Console: colour dummy device 80x30
Calibrating delay loop (skipped), value calculated using timer frequency.. 25.00                                          BogoMIPS (lpj=125000)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 512
CPU: Testing write buffer coherency: ok
CPU0: update cpu_power 1024
CPU0: thread -1, cpu 0, socket 15, mpidr 80000f00 Setting up static identity map for 0x8044a3c0 - 0x8044a3f4
CPU1: Booted secondary processor
CPU1: update cpu_power 1024
CPU1: thread -1, cpu 1, socket 15, mpidr 80000f01 Brought up 2 CPUs
SMP: Total of 2 processors activated.
CPU: All CPU(s) started in HYP mode.
CPU: Virtualization extensions available.
devtmpfs: initialized
VFP support v0.3: implementor 41 architecture 2 part 30 variant 7 rev 5
NET: Registered protocol family 16
DMA: preallocated 256 KiB pool for atomic coherent allocations
cpuidle: using governor ladder
cpuidle: using governor menu
OF: no ranges; cannot translate
OF: no ranges; cannot translate
bio: create slab <bio-0> at 0
vgaarb: loaded
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb i2c i2c-0: IMX I2C adapter registered i2c i2c-1: IMX I2C adapter registered
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@l                                         inux.it>
PTP clock support registered
fsl-ifc 1530000.ifc: Freescale Integrated Flash Controller Switched to clocksource arch_sys_counter
NET: Registered protocol family 2
TCP established hash table entries: 8192 (order: 4, 65536 bytes) TCP bind hash table entries: 8192 (order: 5, 163840 bytes)
TCP: Hash tables configured (established 8192 bind 8192)
TCP: reno registered
UDP hash table entries: 512 (order: 2, 24576 bytes) UDP-Lite hash table entries: 512 (order: 2, 24576 bytes)
NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
Trying to unpack rootfs image as initramfs...
rootfs image is not initramfs (no cpio magic); looks like an initrd Freeing initrd memory: 18700K (88001000 - 89244000)
NFS: Registering the id_resolver key type Key type id_resolver registered Key type id_legacy registered
jffs2: version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
msgmni has been set to 2017
io scheduler noop registered
io scheduler deadline registered
io scheduler cfq registered (default)
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
21c0500.serial: ttyS0 at MMIO 0x21c0500 (irq = 118, base_baud = 9375000) is a 16                                         550A
console [ttyS0] enabled
21c0600.serial: ttyS1 at MMIO 0x21c0600 (irq = 118, base_baud = 9375000) is a 16                                         550A
serial: Freescale lpuart driver
2950000.serial: ttyLP0 at MMIO 0x2950000 (irq = 112, base_baud = 6250000) is a F                                         SL_LPUART
brd: module loaded
loop: module loaded
60000000.nor: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x00008                                         9 Chip ID 0x00227e
Amd/Fujitsu Extended Query Table at 0x0040
  Amd/Fujitsu Extended Query version 1.3.
number of CFI chips: 1
4 ofpart partitions found on MTD device 60000000.nor Creating 4 MTD partitions on "60000000.nor":
0x000000000000-0x000000020000 : "NOR RCW Image"
0x000000020000-0x000000120000 : "NOR DTB Image"
0x000000120000-0x000000920000 : "NOR Linux Kernel Image"
0x000000920000-0x000003b20000 : "NOR Ramdisk Root File System"
6Freescale DSPI master initialized
CAN device driver interface
flexcan 2a70000.can: no ipg clock defined
flexcan: probe of 2a70000.can failed with error -2 flexcan 2a80000.can: no ipg clock defined
flexcan: probe of 2a80000.can failed with error -2
libphy: Freescale PowerQUICC MII Bus: probed mdio_bus mdio@2d24000: cannot get PHY at address 8 fsl-gianfar ethernet.6: enabled errata workarounds, flags: 0x4 fsl-gianfar ethernet.6 eth0: mac: 00:04:9f:ef:00:00 fsl-gianfar ethernet.6 eth0: Running with NAPI enabled fsl-gianfar ethernet.6 eth0: RX BD ring size for Q[0]: 256 fsl-gianfar ethernet.6 eth0: TX BD ring size for Q[0]: 256 fsl-gianfar ethernet.7: enabled errata workarounds, flags: 0x4 fsl-gianfar ethernet.7 eth1: mac: 00:04:9f:ef:01:01 fsl-gianfar ethernet.7 eth1: Running with NAPI enabled fsl-gianfar ethernet.7 eth1: RX BD ring size for Q[0]: 256 fsl-gianfar ethernet.7 eth1: TX BD ring size for Q[0]: 256 fsl-gianfar ethernet.8: enabled errata workarounds, flags: 0x4 fsl-gianfar ethernet.8 eth2: mac: 00:04:9f:ef:02:02 fsl-gianfar ethernet.8 eth2: Running with NAPI enabled fsl-gianfar ethernet.8 eth2: RX BD ring size for Q[0]: 256 fsl-gianfar ethernet.8 eth2: TX BD ring size for Q[0]: 256
e1000: Intel(R) PRO/1000 Network Driver - version 7.3.21-k8-NAPI
e1000: Copyright (c) 1999-2006 Intel Corporation.
e1000e: Intel(R) PRO/1000 Network Driver - 2.3.2-k
e1000e: Copyright(c) 1999 - 2013 Intel Corporation.
xhci-hcd xhci-hcd.0.auto: xHCI Host Controller xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 1 xhci-hcd xhci-hcd.0.auto: irq 125, io mem 0x03100000 hub 1-0:1.0: USB hub found hub 1-0:1.0: 1 port detected xhci-hcd xhci-hcd.0.auto: xHCI Host Controller xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 2 hub 2-0:1.0: USB hub found hub 2-0:1.0: 1 port detected
usbcore: registered new interface driver usb-storage
mousedev: PS/2 mouse device common for all mice i2c /dev entries driver
sdhci: Secure Digital Host Controller Interface driver
sdhci: Copyright(c) Pierre Ossman
sdhci-pltfm: SDHCI platform and OF driver helper
mmc0: SDHCI controller on 1560000.esdhc [1560000.esdhc] using ADMA Value in jrstart addr c0c0005c, value prev 0 new f caam 1700000.crypto: Instantiated RNG4 SH0 caam 1700000.crypto: Instantiated RNG4 SH1 caam 1700000.crypto: device ID = 0x0a140300 (Era 67108864) caam 1700000.crypto: job rings = 4, qi = 0 caam algorithms registered in /proc/crypto caam_jr 1710000.jr: registering rng-caam
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 10
sit: IPv6 over IPv4 tunneling driver
NET: Registered protocol family 17
NET: Registered protocol family 15
can: controller area network core (rev 20120528 abi 9)
NET: Registered protocol family 29
can: raw protocol (rev 20120528)
Key type dns_resolver registered
drivers/rtc/hctosys.c: unable to open rtc device (rtc0)
RAMDISK: gzip image found at block 0
usb 1-1: new high-speed USB device number 2 using xhci-hcd hub 1-1:1.0: USB hub found hub 1-1:1.0: 4 ports detected usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
usb 2-1: Parent hub missing LPM exit latency info.  Power management will be imp                                         acted.
hub 2-1:1.0: USB hub found
hub 2-1:1.0: 4 ports detected
VFS: Mounted root (ext2 filesystem) on device 1:0.
devtmpfs: mounted
Freeing unused kernel memory: 188K (8058e000 - 805bd000)
INIT: version 2.88 booting
Starting udev
udevd[108]: starting version 182
Starting Bootlog daemon: bootlogd.
Populating dev cache
Configuring network interfaces... IPv6: ADDRCONF(NETDEV_UP): eth0: link is not r                                         eady
udhcpc (v1.21.1) started
Sending discover...
libphy: mdio@2d24000:02 - Link is Down
Sending discover...
libphy: mdio@2d24000:02 - Link is Up - 1000/Full
IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready Sending discover...
Sending select for 10.193.20.30...
Lease of 10.193.20.30 obtained, lease time 14400
/etc/udhcpc.d/50default: Adding DNS 10.192.130.201
/etc/udhcpc.d/50default: Adding DNS 10.211.0.3
/etc/udhcpc.d/50default: Adding DNS 10.196.51.200 done.
net.ipv4.conf.default.rp_filter = 1
net.ipv4.conf.all.rp_filter = 1
hwclock: can't open '/dev/misc/rtc': No such file or directory Wed Jun 25 11:59:00 UTC 2014
hwclock: can't open '/dev/misc/rtc': No such file or directory Running postinst /etc/rpm-postinsts/100-sysvinit-inittab...
Running postinst /etc/rpm-postinsts/101-debianutils...
update-rc.d: /etc/init.d/run-postinsts exists during rc.d purge (continuing)  Removing any system startup links for run-postinsts ...
  /etc/rcS.d/S99run-postinsts
INIT: Entering runlevel: 5
Starting OpenBSD Secure Shell server: sshd
  generating ssh RSA key...
  generating ssh ECDSA key...
  generating ssh DSA key...

done.
hwclock: can't open '/dev/misc/rtc': No such file or directory Starting network benchmark server: netserver.
Starting system log daemon...0
Starting kernel log daemon...0
Starting internet superserver: xinetd.
Stopping Bootlog daemon: bootlogd.

Poky (Yocto Project Reference Distro) 1.5 ls1021atwr /dev/ttyS0

ls1021atwr login: root
root@ls1021atwr:~# i2cset -f -y 1 0x52 0x30 0xb0 0xb1 0xb2 0xb3 0xb4 0xb5 0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf i
root@ls1021atwr:~# i2cset -f -y 1 0x52 0x30 0xb0 0xb1 0xb2 0xb3 0xb4 0xb5 0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf i
root@ls1021atwr:~# i2cset -f -y 1 0x52 0x30 0xb0 0xb1 0xb2 0xb3 0xb4 0xb5 0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf i
root@ls1021atwr:~# i2cdump -y 1 0x52 i
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 10 11 12 13 14 15 16 17 17 38 0a 0b 0c 0d 0e 0f    ?????????8??????
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f    ????????????????
20: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f    .???????????????
30: b0 b1 b2 b3 b4 b5 b6 b7 b8 b9 ba bb bc bd be bf    ????????????????
40: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
60: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
70: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
root@ls1021atwr:~# i2cdump -y 1 0x52 i
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 10 11 12 13 14 15 16 17 17 38 0a 0b 0c 0d 0e 0f    ?????????8??????
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f    ????????????????
20: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f    .???????????????
30: b0 b1 b2 b3 b4 b5 b6 b7 b8 b9 ba bb bc bd be bf    ????????????????
40: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
60: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
70: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
root@ls1021atwr:~# i2cdump -y 1 0x52 i
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 10 11 12 13 14 15 16 17 17 38 0a 0b 0c 0d 0e 0f    ?????????8??????
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f    ????????????????
20: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f    .???????????????
30: b0 b1 b2 b3 b4 b5 b6 b7 b8 b9 ba bb bc bd be bf    ????????????????
40: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
60: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
70: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
root@ls1021atwr:~# i2cdump -y 1 0x52 i
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 10 11 12 13 14 15 16 17 17 38 0a 0b 0c 0d 0e 0f    ?????????8??????
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f    ????????????????
20: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f    .???????????????
30: b0 b1 b2 b3 b4 b5 b6 b7 b8 b9 ba bb bc bd be bf    ????????????????
40: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
60: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
70: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
root@ls1021atwr:~#



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23  8:24 [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
@ 2014-07-23  8:24 ` Yuan Yao
  2014-07-23  9:48   ` Lothar Waßmann
  2014-07-23  8:24 ` [PATCH v5 2/2] Documentation:add " Yuan Yao
  2014-07-23 12:28 ` [PATCH v5 0/2] i2c: add " Marek Vasut
  2 siblings, 1 reply; 13+ messages in thread
From: Yuan Yao @ 2014-07-23  8:24 UTC (permalink / raw)
  To: wsa, marex
  Cc: mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c

Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts node.

Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
---
 drivers/i2c/busses/i2c-imx.c | 377 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 324 insertions(+), 53 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 1d7efa3..a9893c3 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -37,22 +37,27 @@
 /** Includes *******************************************************************
 *******************************************************************************/
 
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/dmapool.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
-#include <linux/delay.h>
 #include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/io.h>
-#include <linux/sched.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 #include <linux/platform_data/i2c-imx.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
 
 /** Defines ********************************************************************
 *******************************************************************************/
@@ -63,6 +68,10 @@
 /* Default value */
 #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
 
+/* enable DMA if transfer byte size is bigger than this threshold */
+#define IMX_I2C_DMA_THRESHOLD	16
+#define IMX_I2C_DMA_TIMEOUT	1000
+
 /* IMX I2C registers:
  * the I2C register offset is different between SoCs,
  * to provid support for all these chips, split the
@@ -88,6 +97,7 @@
 #define I2SR_IBB	0x20
 #define I2SR_IAAS	0x40
 #define I2SR_ICF	0x80
+#define I2CR_DMAEN	0x02
 #define I2CR_RSTA	0x04
 #define I2CR_TXAK	0x08
 #define I2CR_MTX	0x10
@@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
 	unsigned		i2cr_ien_opcode;
 };
 
+struct imx_i2c_dma {
+	struct dma_chan		*chan_tx;
+	struct dma_chan		*chan_rx;
+	struct dma_chan		*chan_using;
+	struct completion	cmd_complete;
+	dma_addr_t		dma_buf;
+	unsigned int		dma_len;
+	unsigned int		dma_transfer_dir;
+	unsigned int		dma_data_dir;
+};
+
 struct imx_i2c_struct {
 	struct i2c_adapter	adapter;
 	struct clk		*clk;
@@ -184,6 +205,8 @@ struct imx_i2c_struct {
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
 	const struct imx_i2c_hwdata	*hwdata;
+
+	struct imx_i2c_dma	*dma;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
@@ -254,6 +277,133 @@ static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
 	return readb(i2c_imx->base + (reg << i2c_imx->hwdata->regshift));
 }
 
+/* Functions for DMA support */
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+						dma_addr_t phy_addr)
+{
+	struct imx_i2c_dma *dma;
+	struct dma_slave_config dma_sconfig;
+	struct device *dev = &i2c_imx->adapter.dev;
+	int ret;
+
+	dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
+	if (!dma) {
+		dev_info(dev, "can't allocate DMA struct\n");
+		return -ENOMEM;
+	}
+
+	dma->chan_tx = dma_request_slave_channel(dev, "tx");
+
+	if (!dma->chan_tx) {
+		dev_info(dev, "DMA tx channel request failed\n");
+		ret = -ENODEV;
+		goto fail_al;
+	}
+
+	dma_sconfig.dst_addr = phy_addr +
+				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_maxburst = 1;
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig);
+	if (ret < 0) {
+		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+		goto fail_tx;
+	}
+
+	dma->chan_rx = dma_request_slave_channel(dev, "rx");
+	if (!dma->chan_rx) {
+		dev_info(dev, "DMA rx channel request failed\n");
+		ret = -ENODEV;
+		goto fail_tx;
+	}
+
+	dma_sconfig.src_addr = phy_addr +
+				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig);
+	if (ret < 0) {
+		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+		goto fail_rx;
+	}
+
+	i2c_imx->dma = dma;
+
+	init_completion(&dma->cmd_complete);
+
+	return 0;
+
+fail_rx:
+	dma_release_channel(dma->chan_rx);
+fail_tx:
+	dma_release_channel(dma->chan_tx);
+fail_al:
+	devm_kfree(dev, dma);
+
+	return ret;
+}
+
+static void i2c_imx_dma_callback(void *arg)
+{
+	struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+
+	dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
+			dma->dma_len, dma->dma_data_dir);
+	complete(&dma->cmd_complete);
+}
+
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+					struct i2c_msg *msgs)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_async_tx_descriptor *txdesc;
+	struct device *dev = &i2c_imx->adapter.dev;
+	struct device *chan_dev = dma->chan_using->device->dev;
+
+	dma->dma_buf = dma_map_single(chan_dev, msgs->buf,
+					dma->dma_len, dma->dma_data_dir);
+	if (dma_mapping_error(chan_dev, dma->dma_buf)) {
+		dev_err(dev, "DMA mapping failed\n");
+		return -EINVAL;
+	}
+
+	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
+					dma->dma_len, dma->dma_transfer_dir,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!txdesc) {
+		dev_err(dev, "Not able to get desc for DMA xfer\n");
+		dma_unmap_single(chan_dev, dma->dma_buf,
+					dma->dma_len, dma->dma_data_dir);
+		return -EINVAL;
+	}
+
+	txdesc->callback = i2c_imx_dma_callback;
+	txdesc->callback_param = i2c_imx;
+	dmaengine_submit(txdesc);
+	dma_async_issue_pending(dma->chan_using);
+
+	return 0;
+}
+
+static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+
+	dma->dma_buf = 0;
+	dma->dma_len = 0;
+
+	dma_release_channel(dma->chan_tx);
+	dma->chan_tx = NULL;
+
+	dma_release_channel(dma->chan_rx);
+	dma->chan_rx = NULL;
+
+	dma->chan_using = NULL;
+}
+
 /** Functions for IMX I2C adapter driver ***************************************
 *******************************************************************************/
 
@@ -332,6 +482,11 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 
 	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~I2CR_DMAEN;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
 	return result;
 }
 
@@ -425,44 +580,104 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 
 static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 {
-	int i, result;
+	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
+	unsigned int temp = 0;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct device *dev = &i2c_imx->adapter.dev;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
+	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
 		__func__, msgs->addr << 1);
+	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
+		reinit_completion(&i2c_imx->dma->cmd_complete);
+		dma->chan_using = dma->chan_tx;
+		dma->dma_transfer_dir = DMA_MEM_TO_DEV;
+		dma->dma_data_dir = DMA_TO_DEVICE;
+		dma->dma_len = msgs->len - 1;
+		result = i2c_imx_dma_xfer(i2c_imx, msgs);
+		if (result)
+			return result;
 
-	/* write slave address */
-	imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 
-	/* write data */
-	for (i = 0; i < msgs->len; i++) {
-		dev_dbg(&i2c_imx->adapter.dev,
-			"<%s> write byte: B%d=0x%X\n",
-			__func__, i, msgs->buf[i]);
-		imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
+		/*
+		 * Write slave address.
+		 * The first byte muse be transmitted by the CPU.
+		 */
+		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
+		result = wait_for_completion_interruptible_timeout(
+					&i2c_imx->dma->cmd_complete,
+					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
+		if (result <= 0) {
+			dmaengine_terminate_all(dma->chan_using);
+			if (result)
+				return result;
+			else
+				return -ETIMEDOUT;
+		}
+
+		/* Waiting for Transfer complete. */
+		while (timeout--) {
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+			if (temp & I2SR_ICF)
+				break;
+			udelay(10);
+		}
+
+		if (!timeout)
+			return -ETIMEDOUT;
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp &= ~I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		/* The last data byte must be transferred by the CPU. */
+		imx_i2c_write_reg(msgs->buf[msgs->len-1],
+					i2c_imx, IMX_I2C_I2DR);
 		result = i2c_imx_trx_complete(i2c_imx);
 		if (result)
 			return result;
+
 		result = i2c_imx_acked(i2c_imx);
 		if (result)
 			return result;
+	} else {
+		/* write slave address */
+		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+
+		result = i2c_imx_acked(i2c_imx);
+		if (result)
+			return result;
+
+		dev_dbg(dev, "<%s> write data\n", __func__);
+
+		/* write data */
+		for (i = 0; i < msgs->len; i++) {
+			dev_dbg(dev, "<%s> write byte: B%d=0x%X\n",
+				__func__, i, msgs->buf[i]);
+			imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
+			result = i2c_imx_trx_complete(i2c_imx);
+			if (result)
+				return result;
+			result = i2c_imx_acked(i2c_imx);
+			if (result)
+				return result;
+		}
 	}
 	return 0;
 }
 
 static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 {
-	int i, result;
+	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
 	unsigned int temp;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct device *dev = &i2c_imx->adapter.dev;
 
-	dev_dbg(&i2c_imx->adapter.dev,
-		"<%s> write slave address: addr=0x%x\n",
+	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
 		__func__, (msgs->addr << 1) | 0x01);
 
 	/* write slave address */
@@ -474,7 +689,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	if (result)
 		return result;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
+	dev_dbg(dev, "<%s> setup bus\n", __func__);
 
 	/* setup bus to read data */
 	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
@@ -484,35 +699,82 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
+	dev_dbg(dev, "<%s> read data\n", __func__);
 
-	/* read data */
-	for (i = 0; i < msgs->len; i++) {
-		result = i2c_imx_trx_complete(i2c_imx);
+	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		reinit_completion(&i2c_imx->dma->cmd_complete);
+		dma->chan_using = dma->chan_rx;
+		dma->dma_transfer_dir = DMA_DEV_TO_MEM;
+		dma->dma_data_dir = DMA_FROM_DEVICE;
+		/* The last two data bytes must be transferred by the CPU. */
+		dma->dma_len = msgs->len - 2;
+		result = i2c_imx_dma_xfer(i2c_imx, msgs);
 		if (result)
 			return result;
-		if (i == (msgs->len - 1)) {
-			/* It must generate STOP before read I2DR to prevent
-			   controller from generating another clock cycle */
-			dev_dbg(&i2c_imx->adapter.dev,
-				"<%s> clear MSTA\n", __func__);
-			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-			temp &= ~(I2CR_MSTA | I2CR_MTX);
-			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
-			i2c_imx_bus_busy(i2c_imx, 0);
-			i2c_imx->stopped = 1;
-		} else if (i == (msgs->len - 2)) {
-			dev_dbg(&i2c_imx->adapter.dev,
-				"<%s> set TXAK\n", __func__);
-			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-			temp |= I2CR_TXAK;
-			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		result = wait_for_completion_interruptible_timeout(
+					&i2c_imx->dma->cmd_complete,
+					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
+		if (result <= 0) {
+			dmaengine_terminate_all(dma->chan_using);
+			if (result)
+				return result;
+			else
+				return -ETIMEDOUT;
 		}
-		msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
-		dev_dbg(&i2c_imx->adapter.dev,
-			"<%s> read byte: B%d=0x%X\n",
-			__func__, i, msgs->buf[i]);
+
+		/* waiting for Transfer complete. */
+		while (timeout--) {
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+			if (temp & I2SR_ICF)
+				break;
+			udelay(10);
+		}
+
+		if (!timeout)
+			return -ETIMEDOUT;
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp &= ~I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	} else {
+		/* read data */
+		for (i = 0; i < msgs->len - 2; i++) {
+			result = i2c_imx_trx_complete(i2c_imx);
+			if (result)
+				return result;
+			msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+			dev_dbg(dev, "<%s> read byte: B%d=0x%X\n",
+				__func__, i, msgs->buf[i]);
+		}
+		result = i2c_imx_trx_complete(i2c_imx);
 	}
+
+	/* read n-1 byte data */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp |= I2CR_TXAK;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+	/* read n byte data */
+	result = i2c_imx_trx_complete(i2c_imx);
+	if (result)
+		return result;
+
+	/*
+	 * It must generate STOP before read I2DR to prevent
+	 * controller from generating another clock cycle
+	 */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~(I2CR_MSTA | I2CR_MTX);
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	i2c_imx_bus_busy(i2c_imx, 0);
+	i2c_imx->stopped = 1;
+	msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+
 	return 0;
 }
 
@@ -599,6 +861,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 	u32 bitrate;
+	u32 phy_addr;
 
 	dev_dbg(&pdev->dev, "<%s>\n", __func__);
 
@@ -609,6 +872,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	phy_addr = res->start;
 	base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
@@ -694,6 +958,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
 		i2c_imx->adapter.name);
 	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
 
+	/* Init DMA config if support*/
+	if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
+		dev_warn(&pdev->dev, "can't request DMA\n");
+
 	return 0;   /* Return OK */
 }
 
@@ -705,6 +973,9 @@ static int i2c_imx_remove(struct platform_device *pdev)
 	dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
 	i2c_del_adapter(&i2c_imx->adapter);
 
+	if (i2c_imx->dma)
+		i2c_imx_dma_free(i2c_imx);
+
 	/* setup chip registers to defaults */
 	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
 	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/2] Documentation:add DMA support for freescale i2c driver
  2014-07-23  8:24 [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
  2014-07-23  8:24 ` [PATCH v5 1/2] " Yuan Yao
@ 2014-07-23  8:24 ` Yuan Yao
  2014-07-23 12:28 ` [PATCH v5 0/2] i2c: add " Marek Vasut
  2 siblings, 0 replies; 13+ messages in thread
From: Yuan Yao @ 2014-07-23  8:24 UTC (permalink / raw)
  To: wsa, marex
  Cc: mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c

Add i2c dts node properties for eDMA support, them depend on the eDMA driver.

Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
---
 Documentation/devicetree/bindings/i2c/i2c-imx.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
index 4a8513e..52d37fd 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
@@ -11,6 +11,8 @@ Required properties:
 Optional properties:
 - clock-frequency : Constains desired I2C/HS-I2C bus clock frequency in Hz.
   The absence of the propoerty indicates the default frequency 100 kHz.
+- dmas: A list of two dma specifiers, one for each entry in dma-names.
+- dma-names: should contain "tx" and "rx".
 
 Examples:
 
@@ -26,3 +28,12 @@ i2c@70038000 { /* HS-I2C on i.MX51 */
 	interrupts = <64>;
 	clock-frequency = <400000>;
 };
+
+i2c0: i2c@40066000 { /* i2c0 on vf610 */
+	compatible = "fsl,vf610-i2c";
+	reg = <0x40066000 0x1000>;
+	interrupts =<0 71 0x04>;
+	dmas = <&edma0 0 50>,
+		<&edma0 0 51>;
+	dma-names = "rx","tx";
+};
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23  8:24 ` [PATCH v5 1/2] " Yuan Yao
@ 2014-07-23  9:48   ` Lothar Waßmann
  2014-07-23 11:11     ` Yao Yuan
  0 siblings, 1 reply; 13+ messages in thread
From: Lothar Waßmann @ 2014-07-23  9:48 UTC (permalink / raw)
  To: Yuan Yao
  Cc: wsa, marex, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

Hi,

Yuan Yao wrote:
> Add dma support for i2c. This function depend on DMA driver.
> You can turn on it by write both the dmas and dma-name properties in dts node.
> 
> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> ---
>  drivers/i2c/busses/i2c-imx.c | 377 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 324 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 1d7efa3..a9893c3 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -37,22 +37,27 @@
>  /** Includes *******************************************************************
>  *******************************************************************************/
>  
> -#include <linux/init.h>
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dmapool.h>
>  #include <linux/errno.h>
>  #include <linux/err.h>
>  #include <linux/interrupt.h>
> -#include <linux/delay.h>
>  #include <linux/i2c.h>
> +#include <linux/init.h>
>  #include <linux/io.h>
> -#include <linux/sched.h>
> -#include <linux/platform_device.h>
> -#include <linux/clk.h>
> -#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_dma.h>
>  #include <linux/platform_data/i2c-imx.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
>  
>  /** Defines ********************************************************************
>  *******************************************************************************/
> @@ -63,6 +68,10 @@
>  /* Default value */
>  #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
>  
> +/* enable DMA if transfer byte size is bigger than this threshold */
> +#define IMX_I2C_DMA_THRESHOLD	16
> +#define IMX_I2C_DMA_TIMEOUT	1000
> +
>  /* IMX I2C registers:
>   * the I2C register offset is different between SoCs,
>   * to provid support for all these chips, split the
> @@ -88,6 +97,7 @@
>  #define I2SR_IBB	0x20
>  #define I2SR_IAAS	0x40
>  #define I2SR_ICF	0x80
> +#define I2CR_DMAEN	0x02
>  #define I2CR_RSTA	0x04
>  #define I2CR_TXAK	0x08
>  #define I2CR_MTX	0x10
> @@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
>  	unsigned		i2cr_ien_opcode;
>  };
>  
> +struct imx_i2c_dma {
> +	struct dma_chan		*chan_tx;
> +	struct dma_chan		*chan_rx;
> +	struct dma_chan		*chan_using;
> +	struct completion	cmd_complete;
> +	dma_addr_t		dma_buf;
> +	unsigned int		dma_len;
> +	unsigned int		dma_transfer_dir;
> +	unsigned int		dma_data_dir;
> +};
> +
>  struct imx_i2c_struct {
>  	struct i2c_adapter	adapter;
>  	struct clk		*clk;
> @@ -184,6 +205,8 @@ struct imx_i2c_struct {
>  	int			stopped;
>  	unsigned int		ifdr; /* IMX_I2C_IFDR */
>  	const struct imx_i2c_hwdata	*hwdata;
> +
> +	struct imx_i2c_dma	*dma;
>  };
>  
>  static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> @@ -254,6 +277,133 @@ static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
>  	return readb(i2c_imx->base + (reg << i2c_imx->hwdata->regshift));
>  }
>  
> +/* Functions for DMA support */
> +static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> +						dma_addr_t phy_addr)
> +{
> +	struct imx_i2c_dma *dma;
> +	struct dma_slave_config dma_sconfig;
> +	struct device *dev = &i2c_imx->adapter.dev;
> +	int ret;
> +
> +	dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
> +	if (!dma) {
> +		dev_info(dev, "can't allocate DMA struct\n");
>
dev_err() is designated to print ERROR messages, though this message
could well be eliminated as the memory subsystem will be noisy enough
if the allocation failed.

> +		return -ENOMEM;
> +	}
> +
> +	dma->chan_tx = dma_request_slave_channel(dev, "tx");
> +
> +	if (!dma->chan_tx) {
> +		dev_info(dev, "DMA tx channel request failed\n");
>
dev_err()

> +		ret = -ENODEV;
> +		goto fail_al;
> +	}
> +
> +	dma_sconfig.dst_addr = phy_addr +
> +				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
> +	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> +	dma_sconfig.dst_maxburst = 1;
> +	dma_sconfig.direction = DMA_MEM_TO_DEV;
> +	ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig);
> +	if (ret < 0) {
> +		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>
dev_err()

> +		goto fail_tx;
> +	}
> +
> +	dma->chan_rx = dma_request_slave_channel(dev, "rx");
> +	if (!dma->chan_rx) {
> +		dev_info(dev, "DMA rx channel request failed\n");
>
dev_err()

> +		ret = -ENODEV;
> +		goto fail_tx;
> +	}
> +
> +	dma_sconfig.src_addr = phy_addr +
> +				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
> +	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> +	dma_sconfig.src_maxburst = 1;
> +	dma_sconfig.direction = DMA_DEV_TO_MEM;
> +	ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig);
> +	if (ret < 0) {
> +		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>
dev_err()

> +		goto fail_rx;
> +	}
> +
> +	i2c_imx->dma = dma;
> +
> +	init_completion(&dma->cmd_complete);
> +
> +	return 0;
> +
> +fail_rx:
> +	dma_release_channel(dma->chan_rx);
> +fail_tx:
> +	dma_release_channel(dma->chan_tx);
> +fail_al:
> +	devm_kfree(dev, dma);
>
No need for this one (that's the whole point of using devm_kzalloc())!

> +	return ret;
> +}
> +
> +static void i2c_imx_dma_callback(void *arg)
> +{
> +	struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +
> +	dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
> +			dma->dma_len, dma->dma_data_dir);
> +	complete(&dma->cmd_complete);
> +}
> +
> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> +					struct i2c_msg *msgs)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_async_tx_descriptor *txdesc;
> +	struct device *dev = &i2c_imx->adapter.dev;
> +	struct device *chan_dev = dma->chan_using->device->dev;
> +
> +	dma->dma_buf = dma_map_single(chan_dev, msgs->buf,
> +					dma->dma_len, dma->dma_data_dir);
> +	if (dma_mapping_error(chan_dev, dma->dma_buf)) {
> +		dev_err(dev, "DMA mapping failed\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
> +					dma->dma_len, dma->dma_transfer_dir,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!txdesc) {
> +		dev_err(dev, "Not able to get desc for DMA xfer\n");
>
s/Not able/Unable/

> +		dma_unmap_single(chan_dev, dma->dma_buf,
> +					dma->dma_len, dma->dma_data_dir);
> +		return -EINVAL;
> +	}
> +
> +	txdesc->callback = i2c_imx_dma_callback;
> +	txdesc->callback_param = i2c_imx;
> +	dmaengine_submit(txdesc);
> +	dma_async_issue_pending(dma->chan_using);
> +
> +	return 0;
> +}
> +
> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +
> +	dma->dma_buf = 0;
> +	dma->dma_len = 0;
> +
> +	dma_release_channel(dma->chan_tx);
> +	dma->chan_tx = NULL;
> +
> +	dma_release_channel(dma->chan_rx);
> +	dma->chan_rx = NULL;
> +
> +	dma->chan_using = NULL;
> +}
> +
>  /** Functions for IMX I2C adapter driver ***************************************
>  *******************************************************************************/
>  
> @@ -332,6 +482,11 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>  
>  	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>  	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>
Why reread the register you have written just before?

> +	temp &= ~I2CR_DMAEN;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
>  	return result;
>  }
>  
> @@ -425,44 +580,104 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
>  
>  static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  {
> -	int i, result;
> +	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
> +	unsigned int temp = 0;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct device *dev = &i2c_imx->adapter.dev;
>  
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
> +	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
>  		__func__, msgs->addr << 1);
> +	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
> +		reinit_completion(&i2c_imx->dma->cmd_complete);
> +		dma->chan_using = dma->chan_tx;
> +		dma->dma_transfer_dir = DMA_MEM_TO_DEV;
> +		dma->dma_data_dir = DMA_TO_DEVICE;
> +		dma->dma_len = msgs->len - 1;
> +		result = i2c_imx_dma_xfer(i2c_imx, msgs);
> +		if (result)
> +			return result;
>  
> -	/* write slave address */
> -	imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
> -	result = i2c_imx_trx_complete(i2c_imx);
> -	if (result)
> -		return result;
> -	result = i2c_imx_acked(i2c_imx);
> -	if (result)
> -		return result;
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__);
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp |= I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>  
> -	/* write data */
> -	for (i = 0; i < msgs->len; i++) {
> -		dev_dbg(&i2c_imx->adapter.dev,
> -			"<%s> write byte: B%d=0x%X\n",
> -			__func__, i, msgs->buf[i]);
> -		imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
> +		/*
> +		 * Write slave address.
> +		 * The first byte muse be transmitted by the CPU.
> +		 */
> +		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
> +		result = wait_for_completion_interruptible_timeout(
> +					&i2c_imx->dma->cmd_complete,
> +					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
> +		if (result <= 0) {
> +			dmaengine_terminate_all(dma->chan_using);
> +			if (result)
> +				return result;
> +			else
> +				return -ETIMEDOUT;
> +		}
> +
> +		/* Waiting for Transfer complete. */
> +		while (timeout--) {
> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +			if (temp & I2SR_ICF)
> +				break;
> +			udelay(10);
> +		}
> +
> +		if (!timeout)
> +			return -ETIMEDOUT;
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp &= ~I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		/* The last data byte must be transferred by the CPU. */
> +		imx_i2c_write_reg(msgs->buf[msgs->len-1],
> +					i2c_imx, IMX_I2C_I2DR);
>  		result = i2c_imx_trx_complete(i2c_imx);
>  		if (result)
>  			return result;
> +
>  		result = i2c_imx_acked(i2c_imx);
>  		if (result)
>  			return result;
> +	} else {
> +		/* write slave address */
> +		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
> +		result = i2c_imx_trx_complete(i2c_imx);
> +		if (result)
> +			return result;
> +
> +		result = i2c_imx_acked(i2c_imx);
> +		if (result)
> +			return result;
> +
> +		dev_dbg(dev, "<%s> write data\n", __func__);
> +
> +		/* write data */
> +		for (i = 0; i < msgs->len; i++) {
> +			dev_dbg(dev, "<%s> write byte: B%d=0x%X\n",
> +				__func__, i, msgs->buf[i]);
> +			imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
> +			result = i2c_imx_trx_complete(i2c_imx);
> +			if (result)
> +				return result;
> +			result = i2c_imx_acked(i2c_imx);
> +			if (result)
> +				return result;
> +		}
>  	}
>  	return 0;
>  }
>  
>  static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  {
> -	int i, result;
> +	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>  	unsigned int temp;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct device *dev = &i2c_imx->adapter.dev;
>  
> -	dev_dbg(&i2c_imx->adapter.dev,
> -		"<%s> write slave address: addr=0x%x\n",
> +	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
>  		__func__, (msgs->addr << 1) | 0x01);
>  
>  	/* write slave address */
> @@ -474,7 +689,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  	if (result)
>  		return result;
>  
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
> +	dev_dbg(dev, "<%s> setup bus\n", __func__);
>  
>  	/* setup bus to read data */
>  	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> @@ -484,35 +699,82 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>  	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
>  
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
> +	dev_dbg(dev, "<%s> read data\n", __func__);
>  
> -	/* read data */
> -	for (i = 0; i < msgs->len; i++) {
> -		result = i2c_imx_trx_complete(i2c_imx);
> +	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp |= I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		reinit_completion(&i2c_imx->dma->cmd_complete);
> +		dma->chan_using = dma->chan_rx;
> +		dma->dma_transfer_dir = DMA_DEV_TO_MEM;
> +		dma->dma_data_dir = DMA_FROM_DEVICE;
> +		/* The last two data bytes must be transferred by the CPU. */
> +		dma->dma_len = msgs->len - 2;
> +		result = i2c_imx_dma_xfer(i2c_imx, msgs);
>  		if (result)
>  			return result;
> -		if (i == (msgs->len - 1)) {
> -			/* It must generate STOP before read I2DR to prevent
> -			   controller from generating another clock cycle */
> -			dev_dbg(&i2c_imx->adapter.dev,
> -				"<%s> clear MSTA\n", __func__);
> -			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -			temp &= ~(I2CR_MSTA | I2CR_MTX);
> -			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> -			i2c_imx_bus_busy(i2c_imx, 0);
> -			i2c_imx->stopped = 1;
> -		} else if (i == (msgs->len - 2)) {
> -			dev_dbg(&i2c_imx->adapter.dev,
> -				"<%s> set TXAK\n", __func__);
> -			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -			temp |= I2CR_TXAK;
> -			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		result = wait_for_completion_interruptible_timeout(
> +					&i2c_imx->dma->cmd_complete,
> +					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
> +		if (result <= 0) {
> +			dmaengine_terminate_all(dma->chan_using);
> +			if (result)
> +				return result;
> +			else
> +				return -ETIMEDOUT;
>  		}
> -		msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> -		dev_dbg(&i2c_imx->adapter.dev,
> -			"<%s> read byte: B%d=0x%X\n",
> -			__func__, i, msgs->buf[i]);
> +
> +		/* waiting for Transfer complete. */
> +		while (timeout--) {
> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +			if (temp & I2SR_ICF)
> +				break;
> +			udelay(10);
> +		}
> +
> +		if (!timeout)
> +			return -ETIMEDOUT;
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp &= ~I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +	} else {
> +		/* read data */
> +		for (i = 0; i < msgs->len - 2; i++) {
> +			result = i2c_imx_trx_complete(i2c_imx);
> +			if (result)
> +				return result;
> +			msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +			dev_dbg(dev, "<%s> read byte: B%d=0x%X\n",
> +				__func__, i, msgs->buf[i]);
> +		}
> +		result = i2c_imx_trx_complete(i2c_imx);
>  	}
> +
> +	/* read n-1 byte data */
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp |= I2CR_TXAK;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +	/* read n byte data */
> +	result = i2c_imx_trx_complete(i2c_imx);
> +	if (result)
> +		return result;
> +
> +	/*
> +	 * It must generate STOP before read I2DR to prevent
> +	 * controller from generating another clock cycle
> +	 */
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp &= ~(I2CR_MSTA | I2CR_MTX);
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +	i2c_imx_bus_busy(i2c_imx, 0);
> +	i2c_imx->stopped = 1;
> +	msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +
>  	return 0;
>  }
>  
> @@ -599,6 +861,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	int irq, ret;
>  	u32 bitrate;
> +	u32 phy_addr;
>  
>  	dev_dbg(&pdev->dev, "<%s>\n", __func__);
>  
> @@ -609,6 +872,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	phy_addr = res->start;
>
res->start is NOT a u32 type!

>  	base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
> @@ -694,6 +958,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  		i2c_imx->adapter.name);
>  	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>  
> +	/* Init DMA config if support*/
> +	if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
>
If you want a dma_addr_t variable, why do you declare it with a
different type?


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23  9:48   ` Lothar Waßmann
@ 2014-07-23 11:11     ` Yao Yuan
  2014-07-23 11:14       ` Varka Bhadram
  2014-07-23 12:11       ` Lothar Waßmann
  0 siblings, 2 replies; 13+ messages in thread
From: Yao Yuan @ 2014-07-23 11:11 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: wsa, marex, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 929 bytes --]

Hi,

Thanks for your review.

Lothar Waßmann wrote:
> Yuan Yao wrote:
> > Add dma support for i2c. This function depend on DMA driver.
> > You can turn on it by write both the dmas and dma-name properties in dts node.
> >
> > Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> > ---
> >  drivers/i2c/busses/i2c-imx.c | 377
[...]
> > +
> > +fail_rx:
> > +	dma_release_channel(dma->chan_rx);
> > +fail_tx:
> > +	dma_release_channel(dma->chan_tx);
> > +fail_al:
> > +	devm_kfree(dev, dma);
> >
> No need for this one (that's the whole point of using devm_kzalloc())!
> 

When DMA request failed, I2C will switch to PIO mode. So if the failed reason is just like DMA channel request failed. At this time the DMA should free by devm_kfree(). Is it?

[...]
Thanks.
Yuan Yao
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 11:11     ` Yao Yuan
@ 2014-07-23 11:14       ` Varka Bhadram
  2014-07-23 12:15         ` Lothar Waßmann
  2014-07-23 12:11       ` Lothar Waßmann
  1 sibling, 1 reply; 13+ messages in thread
From: Varka Bhadram @ 2014-07-23 11:14 UTC (permalink / raw)
  To: Yao Yuan, Lothar Waßmann
  Cc: wsa, marex, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

On 07/23/2014 04:41 PM, Yao Yuan wrote:
> Hi,
>
> Thanks for your review.
>
> Lothar Waßmann wrote:
>> Yuan Yao wrote:
>>> Add dma support for i2c. This function depend on DMA driver.
>>> You can turn on it by write both the dmas and dma-name properties in dts node.
>>>
>>> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
>>> ---
>>>   drivers/i2c/busses/i2c-imx.c | 377
> [...]
>>> +
>>> +fail_rx:
>>> +	dma_release_channel(dma->chan_rx);
>>> +fail_tx:
>>> +	dma_release_channel(dma->chan_tx);
>>> +fail_al:
>>> +	devm_kfree(dev, dma);
>>>
>> No need for this one (that's the whole point of using devm_kzalloc())!
>>
> When DMA request failed, I2C will switch to PIO mode. So if the failed reason is just like DMA channel request failed. At this time the DMA should free by devm_kfree(). Is it?

If probe failed the memory will be freed automatically because
we are using devm_kzalloc()...

If we use devm_kzalloc() ,no need to free manually on fail...

-- 
Regards,
Varka Bhadram.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 11:11     ` Yao Yuan
  2014-07-23 11:14       ` Varka Bhadram
@ 2014-07-23 12:11       ` Lothar Waßmann
  1 sibling, 0 replies; 13+ messages in thread
From: Lothar Waßmann @ 2014-07-23 12:11 UTC (permalink / raw)
  To: Yao Yuan
  Cc: wsa, marex, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

Hi,

Yao Yuan wrote:
> Hi,
> 
> Thanks for your review.
> 
> Lothar Waßmann wrote:
> > Yuan Yao wrote:
> > > Add dma support for i2c. This function depend on DMA driver.
> > > You can turn on it by write both the dmas and dma-name properties in dts node.
> > >
> > > Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> > > ---
> > >  drivers/i2c/busses/i2c-imx.c | 377
> [...]
> > > +
> > > +fail_rx:
> > > +	dma_release_channel(dma->chan_rx);
> > > +fail_tx:
> > > +	dma_release_channel(dma->chan_tx);
> > > +fail_al:
> > > +	devm_kfree(dev, dma);
> > >
> > No need for this one (that's the whole point of using devm_kzalloc())!
> > 
> 
> When DMA request failed, I2C will switch to PIO mode. So if the failed reason is just like DMA channel request failed. At this time the DMA should free by devm_kfree(). Is it?
> 
OK. I didn't notice that failing DMA support wasn't a showstopper for
the whole driver.
In this case I would remove the 'failed' from the messages inside 
i2c_imx_dma_request() to make them more benign looking and output them
with dev_dbg(), so they can be en-/disabled with CONFIG_I2C_DEBUG_BUS.

What about returning -EPROBE_DEFER in appropriate cases (when there is
a chance that the DMA driver will be probed later than the I2C driver)?


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 11:14       ` Varka Bhadram
@ 2014-07-23 12:15         ` Lothar Waßmann
  2014-07-23 16:52           ` Marek Vasut
  0 siblings, 1 reply; 13+ messages in thread
From: Lothar Waßmann @ 2014-07-23 12:15 UTC (permalink / raw)
  To: Varka Bhadram
  Cc: Yao Yuan, wsa, marex, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

Hi,

Varka Bhadram wrote:
> On 07/23/2014 04:41 PM, Yao Yuan wrote:
> > Hi,
> >
> > Thanks for your review.
> >
> > Lothar Waßmann wrote:
> >> Yuan Yao wrote:
> >>> Add dma support for i2c. This function depend on DMA driver.
> >>> You can turn on it by write both the dmas and dma-name properties in dts node.
> >>>
> >>> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> >>> ---
> >>>   drivers/i2c/busses/i2c-imx.c | 377
> > [...]
> >>> +
> >>> +fail_rx:
> >>> +	dma_release_channel(dma->chan_rx);
> >>> +fail_tx:
> >>> +	dma_release_channel(dma->chan_tx);
> >>> +fail_al:
> >>> +	devm_kfree(dev, dma);
> >>>
> >> No need for this one (that's the whole point of using devm_kzalloc())!
> >>
> > When DMA request failed, I2C will switch to PIO mode. So if the failed reason is just like DMA channel request failed. At this time the DMA should free by devm_kfree(). Is it?
> 
> If probe failed the memory will be freed automatically because
> we are using devm_kzalloc()...
> 
> If we use devm_kzalloc() ,no need to free manually on fail...
> 
Yes, but as Yuan Yao stated, the driver will still work
without DMA but carry around the unecessary allocated imx_i2c_dma
struct.
The devm_kfree() is not in the failure path of the driver's probe()
function, but in the function that tries to initialize the optional DMA
support.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver
  2014-07-23  8:24 [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
  2014-07-23  8:24 ` [PATCH v5 1/2] " Yuan Yao
  2014-07-23  8:24 ` [PATCH v5 2/2] Documentation:add " Yuan Yao
@ 2014-07-23 12:28 ` Marek Vasut
  2014-07-24  3:36   ` Yao Yuan
  2 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2014-07-23 12:28 UTC (permalink / raw)
  To: Yuan Yao
  Cc: wsa, mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c

On Wednesday, July 23, 2014 at 10:24:41 AM, Yuan Yao wrote:
> Changed in v5:
> - add "*chan_dev = dma->chan_using->device->dev" for reduce the call time.

Did you check if the compiler generates different code ?

> - add the test logs.

[...]

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 12:15         ` Lothar Waßmann
@ 2014-07-23 16:52           ` Marek Vasut
  2014-07-31  6:16             ` Yao Yuan
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2014-07-23 16:52 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Varka Bhadram, Yao Yuan, wsa, mark.rutland, shawn.guo,
	linux-kernel, linux-arm-kernel, linux-i2c

On Wednesday, July 23, 2014 at 02:15:02 PM, Lothar Waßmann wrote:
> Hi,
> 
> Varka Bhadram wrote:
> > On 07/23/2014 04:41 PM, Yao Yuan wrote:
> > > Hi,
> > > 
> > > Thanks for your review.
> > > 
> > > Lothar Waßmann wrote:
> > >> Yuan Yao wrote:
> > >>> Add dma support for i2c. This function depend on DMA driver.
> > >>> You can turn on it by write both the dmas and dma-name properties in
> > >>> dts node.
> > >>> 
> > >>> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> > >>> ---
> > >>> 
> > >>>   drivers/i2c/busses/i2c-imx.c | 377
> > > 
> > > [...]
> > > 
> > >>> +
> > >>> +fail_rx:
> > >>> +	dma_release_channel(dma->chan_rx);
> > >>> +fail_tx:
> > >>> +	dma_release_channel(dma->chan_tx);
> > >>> +fail_al:
> > >>> +	devm_kfree(dev, dma);
> > >> 
> > >> No need for this one (that's the whole point of using devm_kzalloc())!
> > > 
> > > When DMA request failed, I2C will switch to PIO mode. So if the failed
> > > reason is just like DMA channel request failed. At this time the DMA
> > > should free by devm_kfree(). Is it?
> > 
> > If probe failed the memory will be freed automatically because
> > we are using devm_kzalloc()...
> > 
> > If we use devm_kzalloc() ,no need to free manually on fail...
> 
> Yes, but as Yuan Yao stated, the driver will still work
> without DMA but carry around the unecessary allocated imx_i2c_dma
> struct.
> The devm_kfree() is not in the failure path of the driver's probe()
> function, but in the function that tries to initialize the optional DMA
> support.

If the DMA fails, I'd just make the entire probe fail. In case you cannot probe 
DMA for your hardware, which is exected to be DMA capable, it means something is 
wrong anyway.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 12:28 ` [PATCH v5 0/2] i2c: add " Marek Vasut
@ 2014-07-24  3:36   ` Yao Yuan
  2014-07-24  4:16     ` Marek Vasut
  0 siblings, 1 reply; 13+ messages in thread
From: Yao Yuan @ 2014-07-24  3:36 UTC (permalink / raw)
  To: Marek Vasut
  Cc: wsa, mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c

Hi,

Marek Vasut wrote:
> On Wednesday, July 23, 2014 at 10:24:41 AM, Yuan Yao wrote:
> > Changed in v5:
> > - add "*chan_dev = dma->chan_using->device->dev" for reduce the call time.
> 
> Did you check if the compiler generates different code ?
> 

Sorry, I didn't compare the assembly code. It's a subtle change. 
As you mentioned the "noodle" before.

Old:
dma_map_single(dma->chan_using->device->dev, ...);
dma_mapping_error(dma->chan_using->device->dev, ...);
dma_unmap_single(dma->chan_using->device->dev, ...);
	
New:
struct device *chan_dev = dma->chan_using->device->dev;
dma_map_single(chan_dev, ...);
dma_mapping_error(chan_dev, ...);
dma_unmap_single(chan_dev, ...);

> > - add the test logs.
> 
> [...]
> 
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver
  2014-07-24  3:36   ` Yao Yuan
@ 2014-07-24  4:16     ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2014-07-24  4:16 UTC (permalink / raw)
  To: Yao Yuan
  Cc: wsa, mark.rutland, shawn.guo, linux-kernel, linux-arm-kernel, linux-i2c

On Thursday, July 24, 2014 at 05:36:34 AM, Yao Yuan wrote:
> Hi,
> 
> Marek Vasut wrote:
> > On Wednesday, July 23, 2014 at 10:24:41 AM, Yuan Yao wrote:
> > > Changed in v5:
> > > - add "*chan_dev = dma->chan_using->device->dev" for reduce the call
> > > time.
> > 
> > Did you check if the compiler generates different code ?
> 
> Sorry, I didn't compare the assembly code. It's a subtle change.
> As you mentioned the "noodle" before.
> 
> Old:
> dma_map_single(dma->chan_using->device->dev, ...);
> dma_mapping_error(dma->chan_using->device->dev, ...);
> dma_unmap_single(dma->chan_using->device->dev, ...);
> 
> New:
> struct device *chan_dev = dma->chan_using->device->dev;
> dma_map_single(chan_dev, ...);
> dma_mapping_error(chan_dev, ...);
> dma_unmap_single(chan_dev, ...);

You should not use optimization and code cleanup interchangably. Thanks for 
clarifying what this is.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-23 16:52           ` Marek Vasut
@ 2014-07-31  6:16             ` Yao Yuan
  0 siblings, 0 replies; 13+ messages in thread
From: Yao Yuan @ 2014-07-31  6:16 UTC (permalink / raw)
  To: Marek Vasut, Lothar Waßmann
  Cc: Varka Bhadram, wsa, mark.rutland, shawn.guo, linux-kernel,
	linux-arm-kernel, linux-i2c

Marek Vasut wrote:
> On Wednesday, July 23, 2014 at 02:15:02 PM, Lothar Waßmann wrote:
> > Hi,
> >
> > Varka Bhadram wrote:
> > > On 07/23/2014 04:41 PM, Yao Yuan wrote:
> > > > Hi,
> > > >
> > > > Thanks for your review.
> > > >
> > > > Lothar Waßmann wrote:
> > > >> Yuan Yao wrote:
> > > >>> Add dma support for i2c. This function depend on DMA driver.
> > > >>> You can turn on it by write both the dmas and dma-name
> > > >>> properties in dts node.
> > > >>>
> > > >>> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> > > >>> ---
> > > >>>
> > > >>>   drivers/i2c/busses/i2c-imx.c | 377
> > > >
> > > > [...]
> > > >
> > > >>> +
> > > >>> +fail_rx:
> > > >>> +	dma_release_channel(dma->chan_rx);
> > > >>> +fail_tx:
> > > >>> +	dma_release_channel(dma->chan_tx);
> > > >>> +fail_al:
> > > >>> +	devm_kfree(dev, dma);
> > > >>
> > > >> No need for this one (that's the whole point of using devm_kzalloc())!
> > > >
> > > > When DMA request failed, I2C will switch to PIO mode. So if the
> > > > failed reason is just like DMA channel request failed. At this
> > > > time the DMA should free by devm_kfree(). Is it?
> > >
> > > If probe failed the memory will be freed automatically because we
> > > are using devm_kzalloc()...
> > >
> > > If we use devm_kzalloc() ,no need to free manually on fail...
> >
> > Yes, but as Yuan Yao stated, the driver will still work without DMA
> > but carry around the unecessary allocated imx_i2c_dma struct.
> > The devm_kfree() is not in the failure path of the driver's probe()
> > function, but in the function that tries to initialize the optional
> > DMA support.
> 
> If the DMA fails, I'd just make the entire probe fail. In case you cannot probe
> DMA for your hardware, which is exected to be DMA capable, it means
> something is wrong anyway.

Yes, but if there is something wrong in dma, I think the i2c is innocent. So I think the error message is necessary but i2c can continue work.


Best regards,
Yuan Yao

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-07-31  6:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-23  8:24 [PATCH v5 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
2014-07-23  8:24 ` [PATCH v5 1/2] " Yuan Yao
2014-07-23  9:48   ` Lothar Waßmann
2014-07-23 11:11     ` Yao Yuan
2014-07-23 11:14       ` Varka Bhadram
2014-07-23 12:15         ` Lothar Waßmann
2014-07-23 16:52           ` Marek Vasut
2014-07-31  6:16             ` Yao Yuan
2014-07-23 12:11       ` Lothar Waßmann
2014-07-23  8:24 ` [PATCH v5 2/2] Documentation:add " Yuan Yao
2014-07-23 12:28 ` [PATCH v5 0/2] i2c: add " Marek Vasut
2014-07-24  3:36   ` Yao Yuan
2014-07-24  4:16     ` Marek Vasut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).