Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Mon, Jun 17, 2013 at 4:06 PM, Arnaud Patard arnaud.pat...@rtp-net.org wrote: Sricharan R r.sricha...@ti.com writes: I hoped to have missed some mails and that people were testing pandabard support with full support but given what I see, the ethernet support is not there yet. This thread is about removing the non-DT boot. I see some contradiction here. Please, look again at what Thomas said in the IGEP thread: breaking existing support is bad and removing non-DT boot for panda with not-working ethernet would exactly to that. +1 Even no any USB support on Sricharan's setting. Yeah, I'm aware that some extra patches are being developped like the omap clocks one: https://patchwork.kernel.org/patch/2541331/ but they don't seem to be in -next. So, again, please, wait that all needed bits are merged mainline before killing non-DT support (or provide 'mixed' support like what is/was done on kirkwood) Agree, please don't be too quick, :-) Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Mon, Jun 17, 2013 at 4:27 PM, Tony Lindgren t...@atomide.com wrote: Too quick? The basic DT based booting has been working for a few years now to some extent on omaps :) I mean it is OK to drop legacy mode now if it won't break old system. Otherwise, it is better to slow down the dropping legacy patch so that the board won't be one brick for us old board users, :-) Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Thu, Jun 13, 2013 at 6:12 PM, Sricharan R r.sricha...@ti.com wrote: On Thursday 13 June 2013 02:51 PM, Sricharan R wrote: Hi Tony, On Wednesday 12 June 2013 10:44 PM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [130612 09:37]: * Ming Lei tom.leim...@gmail.com [130603 08:34]: Hi, On Sat, May 18, 2013 at 3:17 AM, Tony Lindgren t...@atomide.com wrote: We can now boot with device tree. If you don't want to update u-boot, you can boot with appended DTB with the following instructions: 1. Make sure you have the appended DTB support in .config CONFIG_ARM_APPENDED_DTB=y CONFIG_ARM_ATAG_DTB_COMPAT=y CONFIG_ARM_ATAG_DTB_COMPAT_CMDLINE_EXTEND=y 2. Build the zImage $ ARCH=arm CROSS_COMPILE=... make zImage 3. Build the device tree blobs $ ARCH=arm CROSS_COMPILE=... make dtbs 4. Append the correct panda dtb to zImage Depending on your hardware it's omap4-panda.dtb, omap4-panda-a4.dtb or omap4-panda-es.dtb. $ cat arch/arm/boot/zImage arch/arm/boot/dts/omap4-panda-es.dtb /tmp/appended 5. Use mkimage to produce the appended device tree uImage $ mkimage -A arm -O linux -T kernel -C none -a 0x80008000 -e 0x80008000 \ -n Linux -d /tmp/appended /tmp/uImage I followed the above steps and tried devicetree on Pandaboard against 3.10.0-rc3-next-20130528, and the board will hang during boot, but works well with legacy mode. Hardware: Pandaboard A1 dtb: omap4-panda.dtb See 'dmesg' on below link: http://kernel.ubuntu.com/~ming/up/panda-dts.dmesg Hmm looks like it boots to init. Maybe add initcall_debug to the cmdline in case there's some late_initcall that causes the issue. It's probably some trivial issue causing it. Sricharan, maybe give this a quick try if you have the original pandaboard? I only have pandaboard es. Regards, Tony I tried your cleanup branch omap-for-v3.11/cleanup on panda board and it booted to prompt fine. Hardware: Pandaboard A1 dtb: omap4-panda.dtb git pull on linux-next branch was not working though. Ok, tested in linux-next as well and it booted fine with DTB. HW: OMAP4430ES2.1 PANDA A1 version DTB: OMAP4-PANDA.DTB Booted with ramdisk and mmc FS commit c04efed734409f5a44715b54a6ca1b54b0ccf215 Author: Stephen Rothwell s...@canb.auug.org.au Date: Fri Jun 7 16:40:02 2013 +1000 Add linux-next specific files for 20130607 Looks linux-next-20130607 is broken, see below: LD [M] drivers/usb/gadget/g_ncm.o drivers/usb/musb/omap2430.c: In function 'omap2430_probe': drivers/usb/musb/omap2430.c:571:2: error: 'musb_resources' undeclared (first use in this function) drivers/usb/musb/omap2430.c:571:2: note: each undeclared identifier is reported only once for each function it appears in drivers/usb/musb/omap2430.c:571:2: error: bit-field 'anonymous' width not an integer constant drivers/usb/musb/omap2430.c:585:4: error: bit-field 'anonymous' width not an integer constant make[3]: *** [drivers/usb/musb/omap2430.o] Error 1 make[3]: *** Waiting for unfinished jobs make[2]: *** [drivers/usb/musb] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [drivers/usb] Error 2 make: *** [drivers] Error 2 install kernel and modules DEPMOD 3.10.0-rc4-next-20130607+ Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Fri, Jun 14, 2013 at 9:31 PM, Ming Lei tom.leim...@gmail.com wrote: On Thu, Jun 13, 2013 at 6:12 PM, Sricharan R r.sricha...@ti.com wrote: On Thursday 13 June 2013 02:51 PM, Sricharan R wrote: Hi Tony, On Wednesday 12 June 2013 10:44 PM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [130612 09:37]: * Ming Lei tom.leim...@gmail.com [130603 08:34]: Hi, On Sat, May 18, 2013 at 3:17 AM, Tony Lindgren t...@atomide.com wrote: We can now boot with device tree. If you don't want to update u-boot, you can boot with appended DTB with the following instructions: 1. Make sure you have the appended DTB support in .config CONFIG_ARM_APPENDED_DTB=y CONFIG_ARM_ATAG_DTB_COMPAT=y CONFIG_ARM_ATAG_DTB_COMPAT_CMDLINE_EXTEND=y 2. Build the zImage $ ARCH=arm CROSS_COMPILE=... make zImage 3. Build the device tree blobs $ ARCH=arm CROSS_COMPILE=... make dtbs 4. Append the correct panda dtb to zImage Depending on your hardware it's omap4-panda.dtb, omap4-panda-a4.dtb or omap4-panda-es.dtb. $ cat arch/arm/boot/zImage arch/arm/boot/dts/omap4-panda-es.dtb /tmp/appended 5. Use mkimage to produce the appended device tree uImage $ mkimage -A arm -O linux -T kernel -C none -a 0x80008000 -e 0x80008000 \ -n Linux -d /tmp/appended /tmp/uImage I followed the above steps and tried devicetree on Pandaboard against 3.10.0-rc3-next-20130528, and the board will hang during boot, but works well with legacy mode. Hardware: Pandaboard A1 dtb: omap4-panda.dtb See 'dmesg' on below link: http://kernel.ubuntu.com/~ming/up/panda-dts.dmesg Hmm looks like it boots to init. Maybe add initcall_debug to the cmdline in case there's some late_initcall that causes the issue. It's probably some trivial issue causing it. Sricharan, maybe give this a quick try if you have the original pandaboard? I only have pandaboard es. Regards, Tony I tried your cleanup branch omap-for-v3.11/cleanup on panda board and it booted to prompt fine. Hardware: Pandaboard A1 dtb: omap4-panda.dtb git pull on linux-next branch was not working though. Ok, tested in linux-next as well and it booted fine with DTB. HW: OMAP4430ES2.1 PANDA A1 version DTB: OMAP4-PANDA.DTB Booted with ramdisk and mmc FS commit c04efed734409f5a44715b54a6ca1b54b0ccf215 Author: Stephen Rothwell s...@canb.auug.org.au Date: Fri Jun 7 16:40:02 2013 +1000 Add linux-next specific files for 20130607 Looks linux-next-20130607 is broken, see below: LD [M] drivers/usb/gadget/g_ncm.o drivers/usb/musb/omap2430.c: In function 'omap2430_probe': drivers/usb/musb/omap2430.c:571:2: error: 'musb_resources' undeclared (first use in this function) drivers/usb/musb/omap2430.c:571:2: note: each undeclared identifier is reported only once for each function it appears in drivers/usb/musb/omap2430.c:571:2: error: bit-field 'anonymous' width not an integer constant drivers/usb/musb/omap2430.c:585:4: error: bit-field 'anonymous' width not an integer constant make[3]: *** [drivers/usb/musb/omap2430.o] Error 1 make[3]: *** Waiting for unfinished jobs make[2]: *** [drivers/usb/musb] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [drivers/usb] Error 2 make: *** [drivers] Error 2 install kernel and modules DEPMOD 3.10.0-rc4-next-20130607+ Even I disable musb so that kernel building is OK, but the kernel with dtb still can't boot, see attachment 'dmesg' and config. Thanks, -- Ming Lei U-Boot SPL 2012.04 (Apr 24 2012 - 17:50:07) OMAP4430 ES2.1 OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img U-Boot 2012.04 (Apr 24 2012 - 17:50:07) CPU : OMAP4430 ES2.1 Board: OMAP4 Panda I2C: ready DRAM: 1 GiB MMC: OMAP SD/MMC: 0 Using default environment In:serial Out: serial Err: serial Net: No ethernet found. Hit any key to stop autoboot: 3 2 1 0 SD/MMC found on device 0 reading uEnv.txt 619 bytes read Loaded environment from uEnv.txt Importing environment from mmc ... reading uImage-omap 4239591 bytes read reading uInitrd-omap 8464252 bytes read Booting from mmc ... ## Booting kernel from Legacy Image at 8030 ... Image Name: omap4-panda-dtb-test Image Type: ARM Linux Kernel Image (uncompressed) Data Size:4239527 Bytes = 4 MiB Load Address: 80008000 Entry Point: 80008000 Verifying Checksum ... OK ## Loading init Ramdisk from Legacy Image at 8160 ... Image Name: initramfs Image Type: ARM Linux RAMDisk Image (uncompressed) Data Size:8464188 Bytes = 8.1 MiB Load Address: Entry Point: Verifying Checksum ... OK Loading Kernel Image ... OK OK Starting kernel ... [0.00] Booting Linux on physical CPU 0x0 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Initializing cgroup subsys cpuacct [0.00] Linux version 3.10.0-rc4-next-20130607
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Thu, Jun 13, 2013 at 7:05 PM, Tony Lindgren t...@atomide.com wrote: * Sricharan R r.sricha...@ti.com [130613 03:18]: I tried your cleanup branch omap-for-v3.11/cleanup on panda board and it booted to prompt fine. Hardware: Pandaboard A1 dtb: omap4-panda.dtb OK thanks for testing. git pull on linux-next branch was not working though. Ok, tested in linux-next as well and it booted fine with DTB. HW: OMAP4430ES2.1 PANDA A1 version DTB: OMAP4-PANDA.DTB Booted with ramdisk and mmc FS commit c04efed734409f5a44715b54a6ca1b54b0ccf215 Author: Stephen Rothwell s...@canb.auug.org.au Date: Fri Jun 7 16:40:02 2013 +1000 Add linux-next specific files for 20130607 OK thanks for testing that too. I don't think these patches are in linux-next yet. But still good to hear it works. Ming Lei, any further info on your board? OK, I will test the latest next tree to see if my board can work, and post the result. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
Hi, On Sat, May 18, 2013 at 3:17 AM, Tony Lindgren t...@atomide.com wrote: We can now boot with device tree. If you don't want to update u-boot, you can boot with appended DTB with the following instructions: 1. Make sure you have the appended DTB support in .config CONFIG_ARM_APPENDED_DTB=y CONFIG_ARM_ATAG_DTB_COMPAT=y CONFIG_ARM_ATAG_DTB_COMPAT_CMDLINE_EXTEND=y 2. Build the zImage $ ARCH=arm CROSS_COMPILE=... make zImage 3. Build the device tree blobs $ ARCH=arm CROSS_COMPILE=... make dtbs 4. Append the correct panda dtb to zImage Depending on your hardware it's omap4-panda.dtb, omap4-panda-a4.dtb or omap4-panda-es.dtb. $ cat arch/arm/boot/zImage arch/arm/boot/dts/omap4-panda-es.dtb /tmp/appended 5. Use mkimage to produce the appended device tree uImage $ mkimage -A arm -O linux -T kernel -C none -a 0x80008000 -e 0x80008000 \ -n Linux -d /tmp/appended /tmp/uImage I followed the above steps and tried devicetree on Pandaboard against 3.10.0-rc3-next-20130528, and the board will hang during boot, but works well with legacy mode. Hardware: Pandaboard A1 dtb: omap4-panda.dtb See 'dmesg' on below link: http://kernel.ubuntu.com/~ming/up/panda-dts.dmesg Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] bisected: PandaBoard smsc95xx ethernet driver error from USB timeout
On Fri, Mar 22, 2013 at 4:28 AM, Frank Rowand frank.row...@am.sony.com wrote: I play upstream kernel on Pandaboard A1 frequently, looks not see the failure problem before. Maybe the problem is config dependent. If you may share your config file, I'd like to do the test too. 3.9-rc2-20130314 doesn't have the problem observed on my Pandaboard A1, but I only tested booting from MMC, not from NFS. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] bisected: PandaBoard smsc95xx ethernet driver error from USB timeout
Hi Frank, On Thu, Mar 21, 2013 at 11:29 AM, Frank Rowand frank.row...@am.sony.com wrote: I found the problem on 3.6.11, but have not replicated it on 3.9-rcX yet because my config fails to build on 3.9-rc1 and 3.9-rc2. I'll try to work on that issue tomorrow. I play upstream kernel on Pandaboard A1 frequently, looks not see the failure problem before. Maybe the problem is config dependent. If you may share your config file, I'd like to do the test too. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] Device Power: introduce power controller
On Thu, Dec 6, 2012 at 11:46 AM, Jassi Brar jaswinder.si...@linaro.org wrote: The patch can make usb port deal with the 'power controller' only, and make it avoid to deal with regulators/clocks/... directly. I am curious too, except for clocks and voltage supplies (regulators), what other external resources does a chip need ? For example, one indicator LED which doesn't connect to the same power domain might need to be triggered after the power switch state is changed. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] Device Power: introduce power controller
On Thu, Dec 6, 2012 at 12:49 AM, Roger Quadros rog...@ti.com wrote: On 12/03/2012 05:00 AM, Ming Lei wrote: On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. What stops us using struct regulator here? If you have child regulators supplied by a parent supply, isn't that the right semantic already without introducing a whole new thing? Apologies if I missed the point. There are two purposes: One is to hide the implementation details of the power controller because the user doesn't care how it is implemented, maybe clock, regulator, gpio and other platform dependent stuffs involved, so the patch simplify the usage from the view of users. Which user are you talking about? Here it is the usb port device. At least, there are many boards which have hardwired and self-powered usb devices, so in theory they can benefits from the power controller. Maybe only regulator and clock can't be covered completely for other boards. The patch can make usb port deal with the 'power controller' only, and make it avoid to deal with regulators/clocks/... directly. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
-path matching code. Looks Alan has mentioned, there might be no generic way, and any device's name change in the path may make the way fragile. What you have provided is no less fragile if you allow port1 and the ehci-omap.0 to be set from the outside. Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. - How could these literals like port1 etc be nicely provided by Device Tree? In ARM-land there's pressure to eventually eliminate board files completely and pass in everything from dtb. device_asset can neatly grow DT bindings in a generic way, since the footprint in the board file is some IMO, it isn't necessary to expose these assets to device or users, from the view of device or user, which only cares two actions(poweron and poweroff) about the discussed problem. Also it should be better to put these assets into device resource list, instead of introducing them in 'struct device'. From the point of view of allowing it to be reused on different boards / platforms / arches, you are going to have to do something better than have literals in the code. regulators that already have dt bindings and some device_asset structs. Similarly there's pressure for magic code to service a board (rather than SoC) to go elsewhere than the board file. Looks you associate these assets with ehci-omap device, which mightn't be enough, because we need to control port's power for supporting port power off mechanism. Do you have generic way to associate these assets with usb port device and let port use it generally? Yes, you need a parent device pointer (ehci host controller in this case) and the path rhs, but only stuff that is defined by usb stack code. Needing a parent pointer is OK because this stuff only has meaning for hardwired assets on the platform, so the parent device will always be known as a platform_device at boot time anyway. parent device pointer may work on the panda problem, but may not work on other case if one hardwired device is powered by another power domain. So it is not a general solution on usb port power off. The code I'll provide will work the same in sdio or other bus case, just use mmc host controller pointer and the sdio device name the same way. - Shouldn't this take care of enabling and disabling the ULPI PHY clock on Panda too? There's no purpose leaving it running if the one thing the ULPI PHY is connected to is depowered, and when you do power it, on Panda you will reset the ULPI PHY at the same time anyway (smsc reset and ULPI PHY reset are connected together on Panda). Then you can eliminate omap4_ehci_init() in the board file. OK, we can include the ULPI PHY clock things easily in -power_on() and -power_off() of 'power controller' Yes if the ARM people will accept establishing more code in board files that doesn't have a DT story. As I explained above, it is reasonable to put the power control code in board file, but current DT could convert these board file into device node? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Tue, Dec 4, 2012 at 1:09 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, IMO, it shouldn't be associated with ehci-omap controller driver because the LAN95xx is a external USB device and should be nothing to do with ehci, but it is reasonable to be associated with the hub port because it takes a out of band power control method. offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. Alos, the same ehci-omap driver and same LAN95xx chip is used in beagle board and panda board with different power control approach, does port driver can distinguish these two cases? Port device is a general device(not platform device), how does port driver get platform/board dependent info? 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. Not only regulators involved, clock or other things might be involved too. Also the same power domain might be shared with more than one port, so it is better to introduce power domain to the problem. Looks generic_pm_domain is overkill, so I introduced power controller which only focuses on power on/off and being shared by multiple devices. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/5] USB: prepare for support port power off on non-ACPI device
Hi, This patch set trys to prepare for support of usb port power off mechanism on non-ACPI devices. The port power off mechasnism has been discussed for some time[1][2], and support for ACPI devices has been in shape: - usb port device is introduced - port connect type is introduced and set via ACPI - usb root port power can be switched on/off via ACPI The above has been merged to upstream, and Tianyu is pushing patches[3] to enable auto port power feature. So it is time to discuss how to support it for non-ACPI usb devices, and there are many embedded system in which some of usb devices are hardwired(often self-powered) into board, such as, beagle board, pandaboard, Arndaleboard, etc. So powering on/off these devices may involve platform dependent way, just like ACPI power control is used in Tianyu's patch. This patch set introduces power controller to hide the power switch implementation detail to usb subsytem, in theroy it can be used to other devices and subsystem, and the power controller instance is stored as device's resource. So the power controller can be used to power on/off the power supply from the usb port, and makes support of port power off possible on non-ACPI devices. Associating power controller with one device(such as usb port) which providing power logically is not an easy thing, because platform dependent knowledge(port topology, hardware power domain on the port, ...) has to be applied and the usb port device isn't a platform device and it is created during hub probe(). So looks there is no general approach to do it. This patchset introduces one global device ADD/DEL notifier in driver core, so when a new device is added, one match function in platform code is called to check if the device can be associated with the power controller. This patchset implements the match function on OMAP4 based Pandaboard, and defines the power controller to control power for lan95xx hub and ethernet device. Another idea of associating power controller with port device is by introducing usb port driver, and move all this port power control stuff from platform code to port driver, which is just what I think of and looks doable. I'd like to get some feedback about which one is better, then I can do it in next cycle. Also the two things on Pandaboard have been done at the same time: - powering on the two devices can be delayed to the ehci-omap root hub probing. - the regulatores which are used for these two hardwired and self-powered devices are moved out from ehci-omap driver In fact, Andy Green has been working[4][5] on the above two things, but which isn't the main purpose of these patches, and they might be seen as byproduct of the patchset. Inevitably, there are some overlap between Andy's work and this patchset, hope we can compare, discuss and figure out the perfect solution. arch/arm/mach-omap2/board-omap4panda.c | 99 +++- drivers/base/core.c| 21 ++ drivers/base/power/Makefile|1 + drivers/base/power/power_controller.c | 110 drivers/usb/core/hub.c | 31 - drivers/usb/host/ehci-omap.c | 36 --- include/linux/device.h | 13 include/linux/power_controller.h | 48 ++ include/linux/usb/port.h | 16 + kernel/power/Kconfig |6 ++ 10 files changed, 339 insertions(+), 42 deletions(-) Thanks, -- Ming Lei [1], http://marc.info/?t=13481835493r=1w=2 [2], http://marc.info/?t=13430338453r=1w=2 [3], http://marc.info/?l=linux-usbm=135314427413307w=2 [4], http://marc.info/?t=13539339473r=1w=2 [5], http://www.spinics.net/lists/linux-omap/msg83191.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/5] Device Power: introduce power controller
Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. Cc: Rafael J. Wysocki r...@sisk.pl Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/base/power/Makefile |1 + drivers/base/power/power_controller.c | 110 + include/linux/power_controller.h | 48 ++ kernel/power/Kconfig |6 ++ 4 files changed, 165 insertions(+) create mode 100644 drivers/base/power/power_controller.c create mode 100644 include/linux/power_controller.h diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile index 2e58ebb..c08bfc9 100644 --- a/drivers/base/power/Makefile +++ b/drivers/base/power/Makefile @@ -5,5 +5,6 @@ obj-$(CONFIG_PM_TRACE_RTC) += trace.o obj-$(CONFIG_PM_OPP) += opp.o obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o obj-$(CONFIG_HAVE_CLK) += clock_ops.o +obj-$(CONFIG_POWER_CONTROLLER) += power_controller.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG diff --git a/drivers/base/power/power_controller.c b/drivers/base/power/power_controller.c new file mode 100644 index 000..d7671fb --- /dev/null +++ b/drivers/base/power/power_controller.c @@ -0,0 +1,110 @@ +#include linux/init.h +#include linux/kernel.h +#include linux/power_controller.h +#include linux/slab.h +#include linux/err.h +#include linux/export.h + +static DEFINE_MUTEX(pc_lock); + +static void pc_devm_release(struct device *dev, void *res) +{ +} + +static int pc_devm_match(struct device *dev, void *res, void *match_data) +{ + struct pc_dev_data *data = res; + + return (data-magic == (unsigned long)pc_devm_release); +} + +static struct pc_dev_data *dev_pc_data(struct device *dev) +{ + struct pc_dev_data *data; + + data = devres_find(dev, pc_devm_release, pc_devm_match, NULL); + return data; +} + +static int pc_add_devm_data(struct device *dev, struct power_controller *pc, + void *dev_data, int dev_data_size) +{ + struct pc_dev_data *data; + int ret = 0; + + mutex_lock(pc_lock); + + /* each device should only have one power controller resource */ + data = dev_pc_data(dev); + if (data) { + ret = 1; + goto exit; + } + + data = devres_alloc(pc_devm_release, sizeof(struct pc_dev_data) + + dev_data_size, GFP_KERNEL); + if (!data) { + ret = -ENOMEM; + goto exit; + } + + data-magic = (unsigned long)pc_devm_release; + data-pc = pc; + data-dev_data = data[1]; + memcpy(data-dev_data, dev_data, dev_data_size); + devres_add(dev, data); + +exit: + mutex_unlock(pc_lock); + return ret; +} + +static struct power_controller *dev_pc(struct device *dev) +{ + struct pc_dev_data *data = dev_pc_data(dev); + + if (data) + return data-pc; + return NULL; +} + +struct pc_dev_data *dev_pc_get_data(struct device *dev) +{ + struct pc_dev_data *data = dev_pc_data(dev); + return data; +} +EXPORT_SYMBOL(dev_pc_get_data); + +int dev_pc_bind(struct power_controller *pc, struct device *dev, + void *dev_data, int dev_data_size) +{ + return pc_add_devm_data(dev, pc, dev_data, dev_data_size); +} +EXPORT_SYMBOL(dev_pc_bind); + +void dev_pc_unbind(struct power_controller *pc, struct device *dev) +{ +} +EXPORT_SYMBOL(dev_pc_unbind); + +void dev_pc_power_on(struct device *dev) +{ + struct power_controller *pc = dev_pc(dev); + + if (!pc)return; + + if (atomic_inc_return(pc-count) == 1) + pc-power_on(pc, dev); +} +EXPORT_SYMBOL(dev_pc_power_on); + +void dev_pc_power_off(struct device *dev) +{ + struct power_controller *pc = dev_pc(dev); + + if (!pc)return; + + if (!atomic_dec_return(pc-count)) + pc-power_off(pc, dev); +} +EXPORT_SYMBOL(dev_pc_power_off); diff --git a/include/linux/power_controller.h b/include/linux/power_controller.h new file mode 100644 index 000..772f6d7 --- /dev/null +++ b/include/linux/power_controller.h @@ -0,0 +1,48 @@ +#ifndef _LINUX_POWER_CONTROLLER_H +#define _LINUX_POWER_CONTROLLER_H + +#include linux/device.h + +/* + * One power controller provides simple power on and power off. + * + * One power controller can
[RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier
The global device ADD/DEL notifier is introduced so that some platform code can bind some device resource to the device. When this platform code runs, there is no any bus information about the device, so have to resort to the global notifier. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/base/core.c| 21 + include/linux/device.h | 13 + 2 files changed, 34 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index a235085..37f11ff 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg) early_param(sysfs.deprecated, sysfs_deprecated_setup); #endif +/* global device nofifier */ +struct blocking_notifier_head dev_notifier; + int (*platform_notify)(struct device *dev) = NULL; int (*platform_notify_remove)(struct device *dev) = NULL; static struct kobject *dev_kobj; @@ -1041,6 +1044,8 @@ int device_add(struct device *dev) if (platform_notify) platform_notify(dev); + blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_ADD_DEVICE, dev); + error = device_create_file(dev, uevent_attr); if (error) goto attrError; @@ -1228,6 +1233,7 @@ void device_del(struct device *dev) device_pm_remove(dev); driver_deferred_probe_del(dev); + blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_DEL_DEVICE, dev); /* Notify the platform of the removal, in case they * need to do anything... */ @@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device *parent, void *data, int __init devices_init(void) { + BLOCKING_INIT_NOTIFIER_HEAD(dev_notifier); + devices_kset = kset_create_and_add(devices, device_uevent_ops, NULL); if (!devices_kset) return -ENOMEM; @@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct device *dev, } EXPORT_SYMBOL(dev_printk); +/* The notifier should be avoided as far as possible */ +int dev_register_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(dev_notifier, nb); +} +EXPORT_SYMBOL_GPL(dev_register_notifier); + +int dev_unregister_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(dev_notifier, nb); +} +EXPORT_SYMBOL_GPL(dev_unregister_notifier); + #define define_dev_printk_level(func, kern_level) \ int func(const struct device *dev, const char *fmt, ...) \ { \ diff --git a/include/linux/device.h b/include/linux/device.h index 43dcda9..aeb54f6 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type *bus, #define BUS_NOTIFY_UNBOUND_DRIVER 0x0006 /* driver is unbound from the device */ +/* All 2 notifers below get called with the target struct device * + * as an argument. Note that those functions are likely to be called + * with the device lock held in the core, so be careful. + */ +#define DEV_NOTIFY_ADD_DEVICE 0x0001 /* device added */ +#define DEV_NOTIFY_DEL_DEVICE 0x0002 /* device removed */ +extern int dev_register_notifier(struct notifier_block *nb); +extern int dev_unregister_notifier(struct notifier_block *nb); + +extern struct kset *bus_get_kset(struct bus_type *bus); +extern struct klist *bus_get_device_klist(struct bus_type *bus); + + extern struct kset *bus_get_kset(struct bus_type *bus); extern struct klist *bus_get_device_klist(struct bus_type *bus); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 3/5] USB: hub: apply power controller on usb port
This patch applies the power controller on usb port, so that hub driver can power on one port which isn't provided power by bus. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/usb/core/hub.c | 31 --- include/linux/usb/port.h | 16 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 include/linux/usb/port.h diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index a815fd2..f8075d7 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -26,6 +26,8 @@ #include linux/mutex.h #include linux/freezer.h #include linux/random.h +#include linux/power_controller.h +#include linux/usb/port.h #include asm/uaccess.h #include asm/byteorder.h @@ -848,8 +850,15 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) else dev_dbg(hub-intfdev, trying to enable port power on non-switchable hub\n); - for (port1 = 1; port1 = hub-descriptor-bNbrPorts; port1++) + for (port1 = 1; port1 = hub-descriptor-bNbrPorts; port1++) { + struct usb_port *port = hub-ports[port1 - 1]; + struct pc_dev_data *pc_data = dev_pc_get_data(port-dev); + + /* The power supply for this port isn't managed by bus only */ + if (pc_data) + dev_pc_power_on(port-dev); set_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); + } /* Wait at least 100 msec for power to become stable */ delay = max(pgood_delay, (unsigned) 100); @@ -1541,10 +1550,20 @@ static int hub_configure(struct usb_hub *hub, if (hub-has_indicators blinkenlights) hub-indicator [0] = INDICATOR_CYCLE; - for (i = 0; i hdev-maxchild; i++) + for (i = 0; i hdev-maxchild; i++) { if (usb_hub_create_port_device(hub, i + 1) 0) dev_err(hub-intfdev, couldn't create port%d device.\n, i + 1); + else { + struct usb_port *port = hub-ports[i]; + struct pc_dev_data *pc_data = dev_pc_get_data(port-dev); + if (pc_data pc_data-dev_data) { + struct usb_port_power_switch_data *up = + pc_data-dev_data; + usb_set_hub_port_connect_type(hdev, i + 1, up-type); + } + } + } hub_activate(hub, HUB_INIT); return 0; @@ -1587,8 +1606,14 @@ static void hub_disconnect(struct usb_interface *intf) usb_set_intfdata (intf, NULL); - for (i = 0; i hdev-maxchild; i++) + for (i = 0; i hdev-maxchild; i++) { + struct usb_port *port = hub-ports[i]; + struct pc_dev_data *pc_data = dev_pc_get_data(port-dev); + if (pc_data) + dev_pc_power_off(port-dev); + usb_hub_remove_port_device(hub, i + 1); + } hub-hdev-maxchild = 0; if (hub-hdev-speed == USB_SPEED_HIGH) diff --git a/include/linux/usb/port.h b/include/linux/usb/port.h new file mode 100644 index 000..a853d5e --- /dev/null +++ b/include/linux/usb/port.h @@ -0,0 +1,16 @@ +#ifndef __USB_CORE_PORT_H +#define __USB_CORE_PORT_H + +#include linux/usb.h + +/* + * Only used for describing power switch which provide power to + * hardwired self-powered device which attached to the port + */ +struct usb_port_power_switch_data { + int hub_tier; /* root hub is zero, next tier is 1, ... */ + int port_number; + enum usb_port_connect_type type; +}; + +#endif -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
This patch defines power controller for powering on/off LAN95xx USB hub and USB ethernet devices, and implements one match function to associate the power controller with related USB port device. The big problem of this approach is that it depends on the global device ADD/DEL notifier. Another idea of associating power controller with port device is by introducing usb port driver, and move all this port power control stuff from platform code to the port driver, which is just what I think of and looks doable. The problem of the idea is that port driver is per board, so maybe cause lots of platform sort of code to be put under drivers/usb/port/, but this approach can avoid global device ADD/DEL notifier. I'd like to get some feedback about which one is better or other choice, then I may do it in next cycle. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- arch/arm/mach-omap2/board-omap4panda.c | 99 +++- 1 file changed, 96 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c index 5c8e9ce..3183832 100644 --- a/arch/arm/mach-omap2/board-omap4panda.c +++ b/arch/arm/mach-omap2/board-omap4panda.c @@ -32,6 +32,8 @@ #include linux/usb/musb.h #include linux/wl12xx.h #include linux/platform_data/omap-abe-twl6040.h +#include linux/power_controller.h +#include linux/usb/port.h #include asm/hardware/gic.h #include asm/mach-types.h @@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = { { GPIO_HUB_NRESET, GPIOF_OUT_INIT_LOW, hub_nreset }, }; +static void ehci_hub_power_on(struct power_controller *pc, struct device *dev) +{ + gpio_set_value(GPIO_HUB_NRESET, 1); + gpio_set_value(GPIO_HUB_POWER, 1); +} + +static void ehci_hub_power_off(struct power_controller *pc, struct device *dev) +{ + gpio_set_value(GPIO_HUB_NRESET, 0); + gpio_set_value(GPIO_HUB_POWER, 0); +} + +static struct usb_port_power_switch_data root_hub_port_data = { + .hub_tier = 0, + .port_number = 1, + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED, +}; + +static struct usb_port_power_switch_data smsc_hub_port_data = { + .hub_tier = 1, + .port_number = 1, + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED, +}; + +static struct power_controller pc = { + .name = omap_hub_eth_pc, + .count = ATOMIC_INIT(0), + .power_on = ehci_hub_power_on, + .power_off = ehci_hub_power_off, +}; + +static inline int omap_ehci_hub_port(struct device *dev) +{ + /* we expect dev-parent points to ehcd controller */ + if (dev-parent !strcmp(dev_name(dev-parent), ehci-omap.0)) + return 1; + return 0; +} + +static inline int dev_pc_match(struct device *dev) +{ + struct device *anc; + int ret = 0; + + if (likely(strcmp(dev_name(dev), port1))) + goto exit; + + if (dev-parent (anc = dev-parent-parent)) { + if (omap_ehci_hub_port(anc)) { + ret = 1; + goto exit; + } + + /* is it port of lan95xx hub? */ + if ((anc = anc-parent) omap_ehci_hub_port(anc)) { + ret = 2; + goto exit; + } + } +exit: + return ret; +} + +/* + * Notifications of device registration + */ +static int device_notify(struct notifier_block *nb, unsigned long action, void *data) +{ + struct device *dev = data; + int ret; + + switch (action) { + case DEV_NOTIFY_ADD_DEVICE: + ret = dev_pc_match(dev); + if (likely(!ret)) + goto exit; + if (ret == 1) + dev_pc_bind(pc, dev, root_hub_port_data, sizeof(root_hub_port_data)); + else + dev_pc_bind(pc, dev, smsc_hub_port_data, sizeof(smsc_hub_port_data)); + break; + + case DEV_NOTIFY_DEL_DEVICE: + break; + } +exit: + return 0; +} + +static struct notifier_block usb_port_nb = { + .notifier_call = device_notify, +}; + static void __init omap4_ehci_init(void) { int ret; @@ -178,12 +273,10 @@ static void __init omap4_ehci_init(void) gpio_export(GPIO_HUB_POWER, 0); gpio_export(GPIO_HUB_NRESET, 0); - gpio_set_value(GPIO_HUB_NRESET, 1); usbhs_init(usbhs_bdata); - /* enable power to hub */ - gpio_set_value(GPIO_HUB_POWER, 1); + dev_register_notifier(usb_port_nb); } static struct omap_musb_board_data musb_board_data = { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 5/5] usb: omap ehci: remove all regulator control from ehci omap
From: Andy Green andy.gr...@linaro.org This series migrates it to the hub driver as suggested by Felipe Balbi. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Andy Green andy.gr...@linaro.org Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/usb/host/ehci-omap.c | 36 1 file changed, 36 deletions(-) diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index ac17a7c..d25e39e 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -39,7 +39,6 @@ #include linux/platform_device.h #include linux/slab.h #include linux/usb/ulpi.h -#include linux/regulator/consumer.h #include linux/pm_runtime.h #include linux/gpio.h #include linux/clk.h @@ -150,19 +149,6 @@ static int omap_ehci_init(struct usb_hcd *hcd) return rc; } -static void disable_put_regulator( - struct ehci_hcd_omap_platform_data *pdata) -{ - int i; - - for (i = 0 ; i OMAP3_HS_USB_PORTS ; i++) { - if (pdata-regulator[i]) { - regulator_disable(pdata-regulator[i]); - regulator_put(pdata-regulator[i]); - } - } -} - /* configure so an HC device and id are always provided */ /* always called with process context; sleeping is OK */ @@ -176,14 +162,11 @@ static void disable_put_regulator( static int ehci_hcd_omap_probe(struct platform_device *pdev) { struct device *dev = pdev-dev; - struct ehci_hcd_omap_platform_data *pdata = dev-platform_data; struct resource *res; struct usb_hcd *hcd; void __iomem*regs; int ret = -ENODEV; int irq; - int i; - charsupply[7]; if (usb_disabled()) return -ENODEV; @@ -224,23 +207,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) hcd-rsrc_len = resource_size(res); hcd-regs = regs; - /* get ehci regulator and enable */ - for (i = 0 ; i OMAP3_HS_USB_PORTS ; i++) { - if (pdata-port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) { - pdata-regulator[i] = NULL; - continue; - } - snprintf(supply, sizeof(supply), hsusb%d, i); - pdata-regulator[i] = regulator_get(dev, supply); - if (IS_ERR(pdata-regulator[i])) { - pdata-regulator[i] = NULL; - dev_dbg(dev, - failed to get ehci port%d regulator\n, i); - } else { - regulator_enable(pdata-regulator[i]); - } - } - pm_runtime_enable(dev); pm_runtime_get_sync(dev); @@ -266,7 +232,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) return 0; err_pm_runtime: - disable_put_regulator(pdata); pm_runtime_put_sync(dev); usb_put_hcd(hcd); @@ -291,7 +256,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) struct ehci_hcd_omap_platform_data *pdata = dev-platform_data; usb_remove_hcd(hcd); - disable_put_regulator(dev-platform_data); iounmap(hcd-regs); usb_put_hcd(hcd); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] Device Power: introduce power controller
On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. What stops us using struct regulator here? If you have child regulators supplied by a parent supply, isn't that the right semantic already without introducing a whole new thing? Apologies if I missed the point. There are two purposes: One is to hide the implementation details of the power controller because the user doesn't care how it is implemented, maybe clock, regulator, gpio and other platform dependent stuffs involved, so the patch simplify the usage from the view of users. Another is that several users may share one power controller, and the introduced power controller can help users to use it. Also the power controller is stored as device resource, not any new stuff added into 'struct device', and all users of the power controller needn't write code to operate device resource things too. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier
On Mon, Dec 3, 2012 at 12:13 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: The global device ADD/DEL notifier is introduced so that some platform code can bind some device resource to the device. When this platform code runs, there is no any bus information about the device, so have to resort to the global notifier. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- drivers/base/core.c| 21 + include/linux/device.h | 13 + 2 files changed, 34 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index a235085..37f11ff 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg) early_param(sysfs.deprecated, sysfs_deprecated_setup); #endif +/* global device nofifier */ +struct blocking_notifier_head dev_notifier; + int (*platform_notify)(struct device *dev) = NULL; int (*platform_notify_remove)(struct device *dev) = NULL; static struct kobject *dev_kobj; @@ -1041,6 +1044,8 @@ int device_add(struct device *dev) if (platform_notify) platform_notify(dev); + blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_ADD_DEVICE, dev); + error = device_create_file(dev, uevent_attr); if (error) goto attrError; @@ -1228,6 +1233,7 @@ void device_del(struct device *dev) device_pm_remove(dev); driver_deferred_probe_del(dev); + blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_DEL_DEVICE, dev); /* Notify the platform of the removal, in case they * need to do anything... */ @@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device *parent, void *data, int __init devices_init(void) { + BLOCKING_INIT_NOTIFIER_HEAD(dev_notifier); + devices_kset = kset_create_and_add(devices, device_uevent_ops, NULL); if (!devices_kset) return -ENOMEM; @@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct device *dev, } EXPORT_SYMBOL(dev_printk); +/* The notifier should be avoided as far as possible */ +int dev_register_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(dev_notifier, nb); +} +EXPORT_SYMBOL_GPL(dev_register_notifier); + +int dev_unregister_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(dev_notifier, nb); +} +EXPORT_SYMBOL_GPL(dev_unregister_notifier); + #define define_dev_printk_level(func, kern_level) \ int func(const struct device *dev, const char *fmt, ...) \ { \ diff --git a/include/linux/device.h b/include/linux/device.h index 43dcda9..aeb54f6 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type *bus, #define BUS_NOTIFY_UNBOUND_DRIVER 0x0006 /* driver is unbound from the device */ +/* All 2 notifers below get called with the target struct device * + * as an argument. Note that those functions are likely to be called + * with the device lock held in the core, so be careful. + */ +#define DEV_NOTIFY_ADD_DEVICE 0x0001 /* device added */ +#define DEV_NOTIFY_DEL_DEVICE 0x0002 /* device removed */ +extern int dev_register_notifier(struct notifier_block *nb); +extern int dev_unregister_notifier(struct notifier_block *nb); + +extern struct kset *bus_get_kset(struct bus_type *bus); +extern struct klist *bus_get_device_klist(struct bus_type *bus); + + extern struct kset *bus_get_kset(struct bus_type *bus); extern struct klist *bus_get_device_klist(struct bus_type *bus); Device de/registraton time is not necessarily a good choice for the notification point. At boot time, platform_devices may be being registered with no sign of driver and anything getting enabled in the notifier without further filtering (at each notifier...) will then always be enabled the whole session whether in use or not. probe() and remove() are more interesting because at that point the driver is present and we're definitely instantiating the thing or finished with its instantiation, and it's valid for non-usb devices too. In the one case you're servicing in the series, usb hub port, the device is very unusual in that it has no driver. However if you're going to add a global notifier in generic device code, you should have it do the right thing for normal device case so other people can use it... how I solved this for hub port was to simply normalize hub port by introducing a stub driver
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Mon, Dec 3, 2012 at 12:37 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Hi - This patch defines power controller for powering on/off LAN95xx USB hub and USB ethernet devices, and implements one match function to associate the power controller with related USB port device. The big problem of this approach is that it depends on the global device ADD/DEL notifier. Another idea of associating power controller with port device is by introducing usb port driver, and move all this port power control stuff from platform code to the port driver, which is just what I think of and looks doable. The problem of the idea is that port driver is per board, so maybe cause lots of platform sort of code to be put under drivers/usb/port/, but this approach can avoid global device ADD/DEL notifier. I'd like to get some feedback about which one is better or other choice, then I may do it in next cycle. Cc: Andy Green andy.gr...@linaro.org Cc: Roger Quadros rog...@ti.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- arch/arm/mach-omap2/board-omap4panda.c | 99 +++- 1 file changed, 96 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c index 5c8e9ce..3183832 100644 --- a/arch/arm/mach-omap2/board-omap4panda.c +++ b/arch/arm/mach-omap2/board-omap4panda.c @@ -32,6 +32,8 @@ #include linux/usb/musb.h #include linux/wl12xx.h #include linux/platform_data/omap-abe-twl6040.h +#include linux/power_controller.h +#include linux/usb/port.h #include asm/hardware/gic.h #include asm/mach-types.h @@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = { { GPIO_HUB_NRESET, GPIOF_OUT_INIT_LOW, hub_nreset }, }; +static void ehci_hub_power_on(struct power_controller *pc, struct device *dev) +{ + gpio_set_value(GPIO_HUB_NRESET, 1); + gpio_set_value(GPIO_HUB_POWER, 1); +} You should wait a bit after applying power to the smsc chip before letting go of nReset too. In the regulator-based implementation I sent it's handled by delays encoded in the regulator structs. It isn't a big thing about the discussion. If these code is only platform code, we can use gpio or regulator or other thing. +static void ehci_hub_power_off(struct power_controller *pc, struct device *dev) +{ + gpio_set_value(GPIO_HUB_NRESET, 0); + gpio_set_value(GPIO_HUB_POWER, 0); +} + +static struct usb_port_power_switch_data root_hub_port_data = { + .hub_tier = 0, + .port_number = 1, + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED, +}; + +static struct usb_port_power_switch_data smsc_hub_port_data = { + .hub_tier = 1, + .port_number = 1, + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED, +}; + +static struct power_controller pc = { + .name = omap_hub_eth_pc, + .count = ATOMIC_INIT(0), + .power_on = ehci_hub_power_on, + .power_off = ehci_hub_power_off, +}; + +static inline int omap_ehci_hub_port(struct device *dev) +{ + /* we expect dev-parent points to ehcd controller */ + if (dev-parent !strcmp(dev_name(dev-parent), ehci-omap.0)) + return 1; + return 0; +} + +static inline int dev_pc_match(struct device *dev) +{ + struct device *anc; + int ret = 0; + + if (likely(strcmp(dev_name(dev), port1))) + goto exit; + + if (dev-parent (anc = dev-parent-parent)) { + if (omap_ehci_hub_port(anc)) { + ret = 1; + goto exit; + } + + /* is it port of lan95xx hub? */ + if ((anc = anc-parent) omap_ehci_hub_port(anc)) { + ret = 2; + goto exit; + } + } +exit: + return ret; +} + +/* + * Notifications of device registration + */ +static int device_notify(struct notifier_block *nb, unsigned long action, void *data) +{ + struct device *dev = data; + int ret; + + switch (action) { + case DEV_NOTIFY_ADD_DEVICE: + ret = dev_pc_match(dev); + if (likely(!ret)) + goto exit; + if (ret == 1) + dev_pc_bind(pc, dev, root_hub_port_data, sizeof(root_hub_port_data)); + else + dev_pc_bind(pc, dev, smsc_hub_port_data, sizeof(smsc_hub_port_data)); + break; + + case DEV_NOTIFY_DEL_DEVICE: + break; + } +exit: + return 0; +} + +static struct notifier_block usb_port_nb = { + .notifier_call = device_notify, +}; + Some thoughts on trying to make this functionality specific to power
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Thu, Nov 29, 2012 at 12:43 AM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Nov 2012, Roger Quadros wrote: board file: static struct regulator myreg = { .name = mydevice-regulator, }; static struct device_asset mydevice_assets[] = { { .name = mydevice-regulator, .handler = regulator_default_asset_handler, }, { } }; static struct platform_device mydevice = { ... .dev = { .assets = mydevice_assets, }, ... }; From Pandaboard's point of view, is mydevice supposed to be referring to ehci-omap, LAN95xx or something else? ehci-omap.0. Strictly speaking, the regulator doesn't belongs neither to ehci-omap nor LAN95xx. It belongs to a power domain on the board. And user should have control to switch it OFF when required without hampering operation of ehci-omap, so that the other USB ports are still usable. That is the one disadvantage of the approach we are discussing. But what API would you use to give the user this control? Neither the GPIO nor the regulator has a struct device, so you can't use sysfs directly. And even if you could, it would probably be a bad idea because the user might turn off power to the LAN95xx while the chip was being used. After Tianyu introduced the power power on/off mechanism, sometimes one port power need to be switched off/on. Embedded system is more power sensitive than PC, sounds we have no reason to reject applying the mechanism on embedded world(non ACPI). Looks better to associate the power domain thing(regulator, clock, ...) with one usb port device in this USB problem. The natural answer is to associate the regulator with the USB port that the LAN95xx is connected to, so that the new port-power mechanism could provide the control you want. Then how should that association be set up? As I suggested in below link, the association can be set up easily with one structure of 'struct port_power_domain'. http://www.spinics.net/lists/linux-omap/msg83158.html Lei Ming provided a partial answer, but his suggestion is tied to USB. If we want to set up the association between usb port and power domain, usb knowledge is required, at least bus info and port topology are needed. One difficulty is the fact that the device(such as usb port) is independent with the 'power domain', for example, the device isn't created(ports of the root hub is created after ehci-omap probe, and port device of non-root hub may depend on powering on the power domain) when defining the regulator things, so could we figure out one general way in theory to describe the associated device with the 'power domain'? Andy has tried the wildcard dev path, and port topology string is introduced in my suggestion, looks both are not general. I admit the suggestion is partial because we still have not a general abstract on 'power domain' in this problem, and once it is ready, the solution might be in a shape at least for USB. In PC world, ACPI might do sort of something of the 'power domain' Maybe we need to create a new thread on this discussion and make more guys involved(PM/USB/driver core/OMAP/). I will study the problem further, and hope I can post something for starting the discussion later. It would be better to have a more general approach. So far nobody has Yes, I agree. IMO, my suggestion is still in the direction to being general, because a general 'power domain' concept is introduced in it, for example the 'struct power_domain' is associated with 'struct port_power_domain'. I understand the same 'power domain' concept should be applied to other device or bus too, and the power associated with this device can be switched off sometimes too for saving power consumption. But still looks specific device/subsystem knowledge is required to set up the association. Alan, so could the above be your concern on a more general approach? Or you hope a more general way(such as, do it in driver core or dev PM core) to associate the 'power domain' with one device(for example, usb port in the LAN95xx problem) too? Or other things? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Wed, Nov 28, 2012 at 12:30 AM, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 27 Nov 2012, Ming Lei wrote: IMO, all matches mean the devices are inside the ehci-omap bus, so the direct/simple way is to enable/disable the regulators in the probe() and remove() of ehci-omap controller driver. When this discussion started, Felipe argued against such an approach. He pointed out that the same chip could be used in other platforms, and then the code added to ehci-omap.c would have to be duplicated in several other places. From Andy's implementation, the 'general' code is to enable/disable the regulators(patch 3/5), I am wondering if it is general enough, so the 'duplicated' code are just several lines of regulator_enable/regulator_disable, which should be implemented in many platform drivers. Also from my intuition, power domain should be involved in the problem, because these hard-wired and self-powered USB devices should have its own power domains, and the ehci-omap driver may enable/disable these power domains before adding the bus. We should have a more generic solution in a more central location, like the device core. For example, each struct platform_device could have a list of resources to be enabled when the device is bound to a driver and disabled when the device is unbound. Such a list could include regulators, clocks, and whatever else people can invent. In this case, when it is created the ehci-omap.0 platform device could get an entry pointing to the LAN95xx's regulator. Then the regulator would automatically be turned on when the platform device is bound to the ehci-omap driver. The LAN95xx's regulator is still platform dependent(even for same LAN95xx USB devices on different platforms(omap, tegra, ..)) , so I am wondering why we don't enable it directly in probe() of ehci-omap.0 platform device? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Wed, Nov 28, 2012 at 1:55 AM, Andy Green andy.gr...@linaro.org wrote: On 11/28/2012 01:22 AM, the mail apparently from Ming Lei included: On Wed, Nov 28, 2012 at 12:30 AM, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 27 Nov 2012, Ming Lei wrote: IMO, all matches mean the devices are inside the ehci-omap bus, so the direct/simple way is to enable/disable the regulators in the probe() and remove() of ehci-omap controller driver. When this discussion started, Felipe argued against such an approach. He pointed out that the same chip could be used in other platforms, and then the code added to ehci-omap.c would have to be duplicated in several other places. From Andy's implementation, the 'general' code is to enable/disable the regulators(patch 3/5), I am wondering if it is general enough, so the 'duplicated' code are just several lines of regulator_enable/regulator_disable, which should be implemented in many platform drivers. Fair enough; my main interest was in the device path side when writing the patches. I stuck enough in one place to confirm the series works on Panda case for power control. So long as it doesn't get obsessed with just regulators some common implementation that seems to be discussed will be better. (BTW let me take this opportunity to thank you for your great urbs-in-flight limiting patch on smsc95xx a while back) Also from my intuition, power domain should be involved in the problem, because these hard-wired and self-powered USB devices should have its own power domains, and the ehci-omap driver may enable/disable these power domains before adding the bus. I don't know enough to have an opinion, but the arrangement on Panda is literally a linear regulator is being controlled by a gpio, which fits into struct regulator model. What else would a power domain for that buy us? One problem is that you are addressing to, another is that we may extend Tianyu's per port power off/on mechanism into non-acpi world. Considered that our discussion has been extended to general case instead of pandaboard only, there might be several bus devices which have different power control method, which should be the idea of power domain. I have a draft idea and let me describe it by a pseudo_patch, see below: diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index ac17a7c..c97538f 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -220,6 +220,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) goto err_io; } + list_for_each_entry(ppd, pdata, port_power_domain) { + usb_unregister_port_pm_domain(hcd-self, ppd); + ppd-port_power.power_on(ppd, on) + } + hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); hcd-regs = regs; @@ -290,6 +295,11 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) struct usb_hcd *hcd = dev_get_drvdata(dev); struct ehci_hcd_omap_platform_data *pdata = dev-platform_data; + list_for_each_entry(ppd, pdata, port_power_domain) { + usb_unregister_port_pm_domain(hcd-self, ppd); + ppd-port_power.power_off(ppd, on) + } + usb_remove_hcd(hcd); disable_put_regulator(dev-platform_data); iounmap(hcd-regs); diff --git a/include/linux/platform_data/usb-omap.h b/include/linux/platform_data/usb-omap.h index 8570bcf..30516c9 100644 --- a/include/linux/platform_data/usb-omap.h +++ b/include/linux/platform_data/usb-omap.h @@ -47,6 +47,8 @@ struct ehci_hcd_omap_platform_data { int reset_gpio_port[OMAP3_HS_USB_PORTS]; struct regulator*regulator[OMAP3_HS_USB_PORTS]; unsignedphy_reset:1; + + struct list_headport_power_domain; }; struct ohci_hcd_omap_platform_data { diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 608050b..89c31c0 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -448,6 +448,30 @@ extern void usb_disconnect(struct usb_device **); extern int usb_get_configuration(struct usb_device *dev); extern void usb_destroy_configuration(struct usb_device *dev); +/* + * Only apply in hardwired self-powered devices in bus + */ +struct port_power_domain { + struct usb_bus *bus; + /* +* physical port location in which the power domain provides power on it, +* the first number is the root hub port, and the second number is the +* port number on the second layer hub, ... +*/ + char port_info[32]; /*N-N-N...*/ + + /* +* struct power_domain should be generic power thing, and should be +* defined in power core +*/ + struct power_domain port_power; +}; + +extern int usb_register_port_pm_domain(struct usb_bus *bus
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Wed, Nov 28, 2012 at 7:06 AM, Ming Lei tom.leim...@gmail.com wrote: Also from my intuition, power domain should be involved in the problem, because these hard-wired and self-powered USB devices should have its own power domains, and the ehci-omap driver may enable/disable these power domains before adding the bus. I don't know enough to have an opinion, but the arrangement on Panda is literally a linear regulator is being controlled by a gpio, which fits into struct regulator model. What else would a power domain for that buy us? One problem is that you are addressing to, another is that we may extend Tianyu's per port power off/on mechanism into non-acpi world. Considered that our discussion has been extended to general case instead of pandaboard only, there might be several bus devices which have different power control method, which should be the idea of power domain. I have a draft idea and let me describe it by a pseudo_patch, see below: Sorry, looks sending it too quick, the below pseudo_patch may be more readable about the idea. diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index ac17a7c..c187a11 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -184,6 +184,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) int irq; int i; charsupply[7]; + struct port_power_domain*ppd; if (usb_disabled()) return -ENODEV; @@ -220,6 +221,17 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) goto err_io; } + /* +* register usb per port power domain and enable power on +* this port, to which only hardwired and self-powered device +* attached. And the platform code should provide the +* port power domain list to the usb host controller driver. +*/ + list_for_each_entry(ppd, pdata-port_power_domain, list) { + usb_register_port_pm_domain(hcd-self, ppd); + usb_enable_port_pm_domain(hcd-self, ppd); + } + hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); hcd-regs = regs; @@ -289,6 +301,12 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) struct device *dev = pdev-dev; struct usb_hcd *hcd = dev_get_drvdata(dev); struct ehci_hcd_omap_platform_data *pdata = dev-platform_data; + struct port_power_domain*ppd; + + list_for_each_entry(ppd, pdata-port_power_domain, list) { + usb_disable_port_pm_domain(hcd-self, ppd); + usb_unregister_port_pm_domain(hcd-self, ppd); + } usb_remove_hcd(hcd); disable_put_regulator(dev-platform_data); diff --git a/include/linux/platform_data/usb-omap.h b/include/linux/platform_data/usb-omap.h index 8570bcf..30516c9 100644 --- a/include/linux/platform_data/usb-omap.h +++ b/include/linux/platform_data/usb-omap.h @@ -47,6 +47,8 @@ struct ehci_hcd_omap_platform_data { int reset_gpio_port[OMAP3_HS_USB_PORTS]; struct regulator*regulator[OMAP3_HS_USB_PORTS]; unsignedphy_reset:1; + + struct list_headport_power_domain; }; struct ohci_hcd_omap_platform_data { diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 608050b..6b86d01 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -448,6 +448,39 @@ extern void usb_disconnect(struct usb_device **); extern int usb_get_configuration(struct usb_device *dev); extern void usb_destroy_configuration(struct usb_device *dev); +/* + * Only used for describing power domain which provide power to + * hardwired self-powered devices + */ +struct port_power_domain { + struct list_head list; + struct usb_bus *bus; + + /* +* physical port path, and the power domain of 'port_power' provides +* power on the device attatched to the last non-zero port(Pn-1) of +* the n-1 tier hub, the first number(P1) is the root hub port in +* the path, and the second number(P2) is the port number on the +* second tier hub, ..., until the last number Pn which is zero always. +*/ + char port_path[32]; /* P1-P2-..Pn-1-Pn */ + + /* +* struct power_domain should be generic power thing, and should be +* defined in device power core, maybe it can reuse some kind of +* current power domain structure. +* +* Many ports can share one same power domain, so make the below field +* as pointer. +*/ + struct power_domain *port_power; +}; + +extern int usb_register_port_pm_domain(struct usb_bus *bus
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Tue, Nov 27, 2012 at 5:07 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 26 Nov 2012, Greg KH wrote: Ah, here's the root of your problem, right? You need a way for your hardware to tell the kernel that you have a regulator attached to a specific device? Using the device path and hard-coding it into the kernel is not the proper way to solve this, sorry. Use some other unique way to describe the hardware, surely the hardware designers couldn't have been that foolish not to provide this [1]? As far as I know, the kernel has no other way to describe devices. What about using partial matches? In this example, instead of doing a wildcard match against /platform/usbhs_omap/ehci-omap.0/usb* IMO, all matches mean the devices are inside the ehci-omap bus, so the direct/simple way is to enable/disable the regulators in the probe() and remove() of ehci-omap controller driver. On the other side, both the two LAN95xx USB devices(hub + ethernet) are simple self-powered device. Just like other self-powered devices, the power should be provided from external world, instead of hub driver itself. And it is doable to power on the devices before creating the specific ehci-omap usb bus inside ehci-omap driver. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ftrace Timer on OMAP3
On Mon, May 28, 2012 at 2:56 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, May 25, 2012 at 20:49:07, Thomas Klute wrote: Hi everyone, we're having some trouble getting useful results from Ftrace on OMAP3 (Gumstix Overo). As you can see in the example output below, function duration can't be displayed with a precision higher than about 30.5 us, which matches the frequency of the 32 kHz platform timer. For OMAP1, using OMAP_MPU_TIMER instead of CONFIG_OMAP_32K_TIMER should provide better results [1], but I couldn't find anything similar for OMAP3. Just setting CONFIG_OMAP_32K_TIMER=N didn't change the results. Did I miss anything, or isn't there any more precise timer? I believe you are using mainline kernel here, if yes, you should be enabling the dmtimer using commandline argument, clocksource=gp_timer. Also you need to pass 'nohlt' to kernel since gp_timer can't be kept when cpu is idled in deep state. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] arm: omap4: support pmu
Hi Jon, On Sat, May 19, 2012 at 2:17 AM, Jon Hunter jon-hun...@ti.com wrote: I was performing the test you mentioned in the above thread to reproduce the problem. However, I was not able to reproduce the issue. Did you receive any confirmation from Dmitry this fixed his issue for oprofile? No, looks Dmitry didn't reply on that thread, but I can reproduce/verify the problem easily, see below. By the way, I did not find too many details about the actual fix in the above thread. It appears to be mapping the interrupt to another channel. Can you clarify what this change is doing? If two same channels are used as trigger out channel, any events may route to both CPU, which can be easily observed that there are many unhandled IRQ in one CPU but pmu is just enabled on another CPU. Using different triger out channels can fix the problem and avoid IRQ flood problem which can be triggered by running the below(high frequency sample mode): perf record -e cycles -F 4 noploop 3 I did see the following kernel dump when running the perf record test. Applying your change did not help. Have you seen this? I am using the linux-omap master branch. No, I don't see the warning after applying the 6 patches against -next tree with the mmc request_irq fix patch. From the below log, looks your PMU doesn't work and perf is driven by hrtimer. [ 199.186859] INFO: rcu_sched self-detected stall on CPU { 1} (t=7680 jiffies) [ 199.194427] [c001c5fc] (unwind_backtrace+0x0/0xf4) from [c00a8788] (__rcu_pending+0x158/0x45c) [ 199.203826] [c00a8788] (__rcu_pending+0x158/0x45c) from [c00a8afc] (rcu_check_callbacks+0x70/0x1ac) [ 199.213653] [c00a8afc] (rcu_check_callbacks+0x70/0x1ac) from [c0051a70] (update_process_times+0x38/0x68) [ 199.223968] [c0051a70] (update_process_times+0x38/0x68) from [c008970c] (tick_sched_timer+0x88/0xd8) [ 199.233917] [c008970c] (tick_sched_timer+0x88/0xd8) from [c0067550] (__run_hrtimer+0x7c/0x1e0) [ 199.243316] [c0067550] (__run_hrtimer+0x7c/0x1e0) from [c0067920] (hrtimer_interrupt+0x108/0x294) [ 199.252990] [c0067920] (hrtimer_interrupt+0x108/0x294) from [c001ad60] (twd_handler+0x34/0x40) [ 199.262359] [c001ad60] (twd_handler+0x34/0x40) from [c00a325c] (handle_percpu_devid_irq+0x8c/0x138) [ 199.272216] [c00a325c] (handle_percpu_devid_irq+0x8c/0x138) from [c00a02e8] (generic_handle_irq+0x34/0x44) [ 199.282714] [c00a02e8] (generic_handle_irq+0x34/0x44) from [c00153c0] (handle_IRQ+0x4c/0xac) [ 199.291900] [c00153c0] (handle_IRQ+0x4c/0xac) from [c0008480] (gic_handle_irq+0x2c/0x60) [ 199.300781] [c0008480] (gic_handle_irq+0x2c/0x60) from [c0482964] (__irq_svc+0x44/0x60) [ 199.309509] Exception stack(0xef217c40 to 0xef217c88) [ 199.314819] 7c40: 00a2 ef0ef540 0202 ef216000 c19c0080 [ 199.323394] 7c60: c1a66d00 ef0ef7ac ef217d54 ef217c88 00a3 c004a380 [ 199.331939] 7c80: 6113 [ 199.335601] [c0482964] (__irq_svc+0x44/0x60) from [c004a380] (__do_softirq+0x64/0x214) [ 199.344268] [c004a380] (__do_softirq+0x64/0x214) from [c004a70c] (irq_exit+0x90/0x98) [ 199.352874] [c004a70c] (irq_exit+0x90/0x98) from [c00153c4] (handle_IRQ+0x50/0xac) [ 199.361145] [c00153c4] (handle_IRQ+0x50/0xac) from [c0008480] (gic_handle_irq+0x2c/0x60) [ 199.369995] [c0008480] (gic_handle_irq+0x2c/0x60) from [c0482964] (__irq_svc+0x44/0x60) [ 199.378753] Exception stack(0xef217cf8 to 0xef217d40) [ 199.384033] 7ce0: 009f 0001 [ 199.392639] 7d00: ef0ef540 ef1254c0 ef073480 c19de118 c19bd6c0 [ 199.401184] 7d20: ef0ef7ac ef217d54 0001 ef217d40 00a0 c0071df8 2113 [ 199.409759] [c0482964] (__irq_svc+0x44/0x60) from [c0071df8] (finish_task_switch+0x4c/0xf0) [ 199.418884] [c0071df8] (finish_task_switch+0x4c/0xf0) from [c0481234] (__schedule+0x410/0x808) [ 199.428283] [c0481234] (__schedule+0x410/0x808) from [c0112274] (pipe_wait+0x58/0x78) [ 199.436859] [c0112274] (pipe_wait+0x58/0x78) from [c0112c3c] (pipe_read+0x454/0x584) [ 199.445343] [c0112c3c] (pipe_read+0x454/0x584) from [c0109220] (do_sync_read+0xac/0xf4) [ 199.454101] [c0109220] (do_sync_read+0xac/0xf4) from [c0109de4] (vfs_read+0xac/0x130) [ 199.462646] [c0109de4] (vfs_read+0xac/0x130) from [c0109f38] (sys_read+0x40/0x70) [ 199.470855] [c0109f38] (sys_read+0x40/0x70) from [c0014300] (ret_fast_syscall+0x0/0x3c) At the end of the test I also saw ... Processed 18048959 events and lost 26 chunks! Check IO/CPU overload! Generally, that is not a problem, you can save the trace into ram fs to avoid it. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: fix gpmc request_irq
Hi, On Fri, May 18, 2012 at 3:19 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: : Are you sure it fails ? [1.778930] GPMC revision 6.0 [1.782226] gpmc: irq-52 could not claim: err -22 Thanks for the reference. removing SHARED flag is not solution and yes, you might have to keep the dev-id if that was the issue. Is the gpmc a shared interrupt line? SHARED is not needed at all for non shared interrupt line in hardware. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: fix gpmc request_irq
On Fri, May 18, 2012 at 3:43 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: Is the gpmc a shared interrupt line? SHARED is not needed at all for non shared interrupt line in hardware. The line is shared. If so, dev_id should be added. But sorry, could you let me know what other interrupt sources are shared with the line? From 17.3.2 Interrupt Requests to Cortex-A9 MPU INTC of OMAP4 TRM, GPMC_IRQ is the only source of MA_IRQ_20 for Cortex-A9 MPU INTC. Even though GPMC_IRQ is connected with D_IRQ_59(DSP INTC) and MA_IRQ_20(MPU INTC), this still means it is not a shared line for MPU INTC. Also looks the above is similar for OMAP3. Correct me if the above is wrong. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: fix gpmc request_irq
Hi, On Fri, May 18, 2012 at 6:13 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: Generally IRQF_SHARED means, the interrupt line is only one but multiple devices can generate the interrupts. If you connect different devices on GMPC using chip selects like Ethernet controller, Nand, NOR contoller, all of they can generate events which can be reported by the GPMC. That's the reason I shared the line is shared. IMO, it depends on if the event handlers of other chips(ethernet, nand, nor..) share the same irq number(GPMC_IRQ). If so, it should be IRQF_SHARED. If they are handled in its own irq number(it may convert to irq dispatching), that shouldn't be IRQF_SHARED. On Fri, May 18, 2012 at 6:51 PM, Mohammed, Afzal af...@ti.com wrote: Hi Ming, Requesting irq is called by driver of IP, while whether it is shared or not depends on SoC where IP lives, so ideally it seems platform should inform the driver whether it is shared driver should do what platform tells. Or else to be safe, it should be made shared always ? Shared or not(IRQF_SHARED) may be determined by hardware and software handling. If the same interrupt line is shared by several peripherals and all interrupt handlers are requested to same interrupt number for handling IRQs from these peripherals, it should be IRQF_SHARED. This may not make much sense now w.r.t gpmc, but would be applicable once gpmc becomes a driver (undergoing conversion), but may not be that important as there are no SoCs presently sharing gpmc interrupt (afaik) Looks the fix isn't needed if so. Anyway, thanks your guys for exposing much info about GPMC irq handling. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP2+: fix gpmc request_irq
The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED is passed to request_irq and dev_id is set as NULL, request_irq will return -EINVAL. This patch just removes the flag of IRQF_SHARED to make the irq registration successful. Cc: Kevin Hilman khil...@ti.com Cc: Tony Lindgren t...@atomide.com Signed-off-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/gpmc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 2286410..e45d31b 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -769,7 +769,7 @@ static int __init gpmc_init(void) irq++; } - ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, gpmc, NULL); + ret = request_irq(gpmc_irq, gpmc_handle_irq, 0, gpmc, NULL); if (ret) pr_err(gpmc: irq-%d could not claim: err %d\n, gpmc_irq, ret); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] ARM: OMAP4: PMU: Add runtime PM support
Jon, On Wed, May 16, 2012 at 6:05 AM, Ming Lei ming@canonical.com wrote: On Tuesday, May 15, 2012, Jon Hunter jon-hun...@ti.com wrote: Hi Ming, On 05/14/2012 11:53 PM, Ming Lei wrote: On Thu, May 10, 2012 at 5:35 AM, Jon Hunter jon-hun...@ti.com wrote: From: Jon Hunter jon-hun...@ti.com This patch is based upon Ming Lei's patch to add runtime PM support for OMAP4 [1]. In Ming's original patch the CTI interrupts were being enabled during runtime when the PMU was used but they were only configured once during init. Therefore move the configuration of the CTI interrupts to the runtime PM functions. As Shilimkar pointed out, you need to give the reason why the change is introduced in the patch. Yes will do. I have been waiting to get some feedback on HW_AUTO versus SW_SLEEP from design before posting a V2. I will enhance the changelog. Looks pandaboard will hang during kernel boot with the latest next tree, so perf can't be tested now. Once the problem is fixed, l will run perf test and provide my feedback. After bisecting, looks it is the 1st commit which triggers kernel boot hang: commit 5f3aa31f77733605f04a5a92ddc475ffd439585f Merge: 0f131ea ce4deaa Author: Stephen Rothwell s...@canb.auug.org.au Date: Mon May 14 18:55:39 2012 +1000 Merge remote-tracking branch 'arm-soc/for-next' These 6 patches can't be applied against previous commit, so I can't verify these patches now. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] arm: omap4: support pmu
On Thu, May 10, 2012 at 5:35 AM, Jon Hunter jon-hun...@ti.com wrote: + + /*configure CTI0 for pmu irq routing*/ + cti_init(omap4_cti[0], base0, OMAP44XX_IRQ_CTI0, 6); + cti_unlock(omap4_cti[0]); + cti_map_trigger(omap4_cti[0], 1, 6, 2); + + /*configure CTI1 for pmu irq routing*/ + cti_init(omap4_cti[1], base1, OMAP44XX_IRQ_CTI1, 6); + cti_unlock(omap4_cti[1]); + cti_map_trigger(omap4_cti[1], 1, 6, 2); As pointed in another thread, the above line should be changed to below: cti_map_trigger(omap4_cti[1], 1, 6, 3); which can avoid irq flood issue at high frequency sample mode. For detailed description about the issue and fix, see below link: http://permalink.gmane.org/gmane.linux.linaro.devel/10532 Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] mmc: omap_hsmmc: pass IRQF_ONESHOT to request_threaded_irq
The flag of IRQF_ONESHOT should be passed to request_threaded_irq, otherwise the following failure message should be dumped because hardware handler is defined as NULL: [3.383483] genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 368 [3.392730] omap_hsmmc: probe of omap_hsmmc.0 failed with error -22 The patch fixes one kernel hang bug which is caused by mmc card probe failure and root device can't be brought up. Signed-off-by: Ming Lei ming@canonical.com --- drivers/mmc/host/omap_hsmmc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index ebaf62a..9a7a60a 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1981,7 +1981,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) ret = request_threaded_irq(mmc_slot(host).card_detect_irq, NULL, omap_hsmmc_detect, - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, mmc_hostname(mmc), host); if (ret) { dev_dbg(mmc_dev(host-mmc), -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] ARM: OMAP4: PMU: Add runtime PM support
On Wed, May 16, 2012 at 4:17 PM, Ming Lei ming@canonical.com wrote: Jon, On Wed, May 16, 2012 at 6:05 AM, Ming Lei ming@canonical.com wrote: On Tuesday, May 15, 2012, Jon Hunter jon-hun...@ti.com wrote: Hi Ming, On 05/14/2012 11:53 PM, Ming Lei wrote: On Thu, May 10, 2012 at 5:35 AM, Jon Hunter jon-hun...@ti.com wrote: From: Jon Hunter jon-hun...@ti.com This patch is based upon Ming Lei's patch to add runtime PM support for OMAP4 [1]. In Ming's original patch the CTI interrupts were being enabled during runtime when the PMU was used but they were only configured once during init. Therefore move the configuration of the CTI interrupts to the runtime PM functions. As Shilimkar pointed out, you need to give the reason why the change is introduced in the patch. Yes will do. I have been waiting to get some feedback on HW_AUTO versus SW_SLEEP from design before posting a V2. I will enhance the changelog. Looks pandaboard will hang during kernel boot with the latest next tree, so perf can't be tested now. Once the problem is fixed, l will run perf test and provide my feedback. After bisecting, looks it is the 1st commit which triggers kernel boot hang: commit 5f3aa31f77733605f04a5a92ddc475ffd439585f Merge: 0f131ea ce4deaa Author: Stephen Rothwell s...@canb.auug.org.au Date: Mon May 14 18:55:39 2012 +1000 Merge remote-tracking branch 'arm-soc/for-next' The above kernel hang is caused by mmc driver probe failure, which can be fixed by the patch in below link: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68848.html After applying the patch, I can test the 6 patches and looks they work well about perf support on omap4, also perf can work well after S2R. Tested-by: Ming Lei ming@canonical.com Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] ARM: OMAP4: PMU: Add runtime PM support
On Thu, May 10, 2012 at 5:35 AM, Jon Hunter jon-hun...@ti.com wrote: From: Jon Hunter jon-hun...@ti.com This patch is based upon Ming Lei's patch to add runtime PM support for OMAP4 [1]. In Ming's original patch the CTI interrupts were being enabled during runtime when the PMU was used but they were only configured once during init. Therefore move the configuration of the CTI interrupts to the runtime PM functions. As Shilimkar pointed out, you need to give the reason why the change is introduced in the patch. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074153.html Cc: Ming Lei ming@canonical.com Cc: Will Deacon will.dea...@arm.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@ti.com Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/devices.c | 50 ++-- 1 files changed, 27 insertions(+), 23 deletions(-) diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index 636533d..b02aa81 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -18,6 +18,7 @@ #include linux/slab.h #include linux/of.h #include linux/platform_data/omap4-keypad.h +#include linux/pm_runtime.h #include mach/hardware.h #include mach/irqs.h @@ -434,13 +435,22 @@ static struct omap_device_pm_latency omap_pmu_latency[] = { }; static struct cti omap4_cti[2]; +static struct platform_device *pmu_dev; static void omap4_enable_cti(int irq) { - if (irq == OMAP44XX_IRQ_CTI0) + pm_runtime_get_sync(pmu_dev-dev); + if (irq == OMAP44XX_IRQ_CTI0) { + /* configure CTI0 for pmu irq routing */ + cti_unlock(omap4_cti[0]); + cti_map_trigger(omap4_cti[0], 1, 6, 2); cti_enable(omap4_cti[0]); - else if (irq == OMAP44XX_IRQ_CTI1) + } else if (irq == OMAP44XX_IRQ_CTI1) { + /* configure CTI1 for pmu irq routing */ + cti_unlock(omap4_cti[1]); + cti_map_trigger(omap4_cti[1], 1, 6, 2); The above line should be changed to below cti_map_trigger(omap4_cti[1], 1, 6, 3); See below link for addressed irq flood issue. http://permalink.gmane.org/gmane.linux.linaro.devel/10532 cti_enable(omap4_cti[1]); + } } static void omap4_disable_cti(int irq) @@ -449,6 +459,7 @@ static void omap4_disable_cti(int irq) cti_disable(omap4_cti[0]); else if (irq == OMAP44XX_IRQ_CTI1) cti_disable(omap4_cti[1]); + pm_runtime_put(pmu_dev-dev); } static irqreturn_t omap4_pmu_handler(int irq, void *dev, irq_handler_t handler) @@ -461,27 +472,20 @@ static irqreturn_t omap4_pmu_handler(int irq, void *dev, irq_handler_t handler) return handler(irq, dev); } -static void __init omap4_configure_pmu_irq(void) +static int __init omap4_configure_pmu(void) { - void __iomem *base0; - void __iomem *base1; + omap4_cti[0].base = ioremap(OMAP44XX_CTI0_BASE, SZ_4K); + omap4_cti[1].base = ioremap(OMAP44XX_CTI1_BASE, SZ_4K); - base0 = ioremap(OMAP44XX_CTI0_BASE, SZ_4K); - base1 = ioremap(OMAP44XX_CTI1_BASE, SZ_4K); - if (!base0 !base1) { + if (!omap4_cti[0].base || !omap4_cti[1].base) { pr_err(ioremap for OMAP4 CTI failed\n); - return; + return -ENOMEM; } - /*configure CTI0 for pmu irq routing*/ - cti_init(omap4_cti[0], base0, OMAP44XX_IRQ_CTI0, 6); - cti_unlock(omap4_cti[0]); - cti_map_trigger(omap4_cti[0], 1, 6, 2); + cti_init(omap4_cti[0], omap4_cti[0].base, OMAP44XX_IRQ_CTI0, 6); + cti_init(omap4_cti[1], omap4_cti[1].base, OMAP44XX_IRQ_CTI1, 6); - /*configure CTI1 for pmu irq routing*/ - cti_init(omap4_cti[1], base1, OMAP44XX_IRQ_CTI1, 6); - cti_unlock(omap4_cti[1]); - cti_map_trigger(omap4_cti[1], 1, 6, 2); + return 0; } static struct platform_device* __init omap4_init_pmu(void) @@ -492,6 +496,9 @@ static struct platform_device* __init omap4_init_pmu(void) struct omap_hwmod* oh[3]; char *dev_name = arm-pmu; + if (omap4_configure_pmu()) + return NULL; + hw = l3_main_3; oh[0] = omap_hwmod_lookup(hw); if (!oh[0]) { @@ -530,14 +537,11 @@ static void __init omap_init_pmu(void) } else if (cpu_is_omap34xx()) { omap_pmu_device.resource = omap3_pmu_resource; } else if (cpu_is_omap44xx()) { - struct platform_device *pd; - - pd = omap4_init_pmu(); - if (!pd) + pmu_dev = omap4_init_pmu(); + if (!pmu_dev) return; - omap_device_enable(od-pdev); - omap4_configure_pmu_irq(); + pm_runtime_enable
Re: oprofile and ARM A9 hardware counter
On Wed, May 9, 2012 at 3:51 AM, Jon Hunter jon-hun...@ti.com wrote: I had to make a couple mods to Ming's patches but I do have something working now that _should_ not break PM. However, per my previous email (in response to Benoit's) I am struggling with the definition of the CLKDM_CAN_XX_AUTO flags to know the correct way to fix this. I was going to send out my patches, but I wanted to get some more feedback on this first. I can test your patch once I return home from business trip next week. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
On Tue, May 1, 2012 at 6:25 AM, Kevin Hilman khil...@ti.com wrote: Unfortunately, digging in the code isn't going to help much. We've been trying to get our heads around some undocumented reset behavior for the various debug IPs in OMAP. After a little digging and clarification from HW designers, it appears that if we allow the EMU clockdomain to idle, a full reset of the debug IPs is done. That means there are two solutions to this problem: 1) don't ever alow EMU clockdomain to idle. 2) modify CTI driver to re-init for every use. Obviously (1) is the easiet, and hacks for that have already been posted, but that has pretty significant impacts on power savings. (2) is the right solution to merge upstream, but needs writing. For (2), using runtime PM methods in the driver would be the simplest here since the -runtime_resume method will be called every time the device is about to be used. The previous patch has supported runtime pm on omap4 pmu[1], but still didn't fix the problem. Could you comment on the patch and point out the proper way to support pmu runtime for fixing the problem? [1], http://marc.info/?l=linux-arm-kernelm=131946777028358w=2 Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 10, 2012 at 5:51 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. Good point. My point is such facilities is already part of the kernel and there is no need to add a new one. The last proposal was allowing user to choose gptimer as a clocksource and then you already have facility to disable C-state now so, all should work in general 'nohlt' in boot cmd should work to prevent CPU from entering deep C-state, which looks enough to make gptimer working well if system suspend isn't considered. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
On Wed, Apr 4, 2012 at 7:29 AM, Paul Walmsley p...@pwsan.com wrote: Hi On Tue, 3 Apr 2012, Kevin Hilman wrote: Indeed, like you, I have to change the EMU clock domain to SWSUP[1] in order to see any interrupts and see anything in perf top. This isn't really a mergeable workaround, so I'll look into this a little closer with Santosh to see what we can do once we fully understand the HW problem. Part of the problem is that the clockdomain data for the emu_sys clockdomain is wrong. Here's something to try to fix it. It might just be enough to get it to work. - Paul From: Paul Walmsley p...@pwsan.com Date: Tue, 3 Apr 2012 17:13:48 -0600 Subject: [PATCH] ARM: OMAP44xx: clockdomain data: correct the emu_sys_clkdm CLKTRCTRL data According to the 4430 ES2.0 TRM vX Table 3-744 CM_EMU_CLKSTCTRL, the emu_sys clockdomain data in mainline is incorrect. The emu_sys clockdomain does not support the DISABLE_AUTO state, and instead it supports the FORCE_WAKEUP state. Signed-off-by: Paul Walmsley p...@pwsan.com Cc: Benoît Cousson b-cous...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/clockdomains44xx_data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..bd7ed13 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags = CLKDM_CAN_HWSUP, + .flags = CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_FORCE_WAKEUP, I tested the patch just now, but unfortunately, the change still doesn't make PMU to generate IRQs. Mark the flags as CLKDM_CAN_SWSUP may work, but PMU will stop producing IRQs after resuming from suspend. Thanks -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( [1], http://marc.info/?l=linux-arm-kernelm=133311647410324w=2 Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sun, Apr 1, 2012 at 3:10 AM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Sat, Mar 31, 2012 at 2:09 PM, Ming Lei tom.leim...@gmail.com wrote: On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( It's not friendly but doable. But above can be supported I guess. Since you are talking about doing menuconfig and rebuilding kernel so what can be done is when one disable CPUIDLE and PM, one can disabled 32K source as well. And then 32K clocksource init should fail and fallback on gptimer clock source. OK, it should work, but looks OMAP_32K_TIMER option has to be kept for the usage, which may be a bit divergent to the purpose of the patch set. So how about introducing a kernel parameter to decide if bypassing 32K source and using gptimer source directly, and let which depend on PM? Vaibhav, Can you update your patch to support above. The patch which I did was doing exactly that but was not using hwmod. The problem with your patch is synctimer hwmod lookup on OMAP will never fail if the hwmod entry is supported. So you might better use failure case of omap_init_clocksource_32k() for fall back on gptimer source as I tried in my patch. That should support You patch still doesn't work for the usage because you removed 32K option. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 5:20 PM, Shilimkar, Santosh santosh.shilim...@ti.com After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. If neither CONFIG_PM or CONFIG_CPUIDLE is set, GPTIMER2 with sysclock should be an option. As I commented before, high resolution timer is very useful for trace, debug or high frequency perf sample. Suppose you want to trace the running time of one function, 32K counter can only give you a resolution of ~35us, which is too coarse to work well. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 21, 2012 at 7:29 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:14:30, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. How about the 3rd option? 3), take the way in your patch 1) at default, but will switch to register dmtimer directly and bypass 32k-counter if user need it via kernel parameter. As far as I can think of, the situations required for dmtimer are high-frequency perf sample and high precision trace points, so looks it is OK to take 32k-counter at default. But if you register both the timers (dmtimer 32ksync), then initially kernel will only pick up dmtimer, as this has better rating. And late in Looks not so, I found that 32ksync is always selected as the default clocksource if both are registered. the boot sequence clocksource switch will happen, base on kernel parameter (clocksource=). So logically dmtimer will be always used as a default here. Not so at least on my Pandaboard. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP3+: fix oops triggered in omap_prcm_register_chain_handler(v1)
This patch fixes the oops below[1]. Obviously, the count of struct irq_chip_generic instances to be allocated and setup should be irq_setup-nr_regs instead of irq_setup-nr_regs plus one, so just fix the iterator to avoid the oops. [1], oops log. [1.790242] Unable to handle kernel NULL pointer dereference at virtual address 0004 [1.798632] pgd = c0004000 [1.801638] [0004] *pgd= [1.805400] Internal error: Oops: 805 [#1] PREEMPT SMP THUMB2 [1.811381] Modules linked in: [1.814601] CPU: 1Not tainted (3.3.0-next-20120320+ #733) [1.820683] PC is at irq_setup_generic_chip+0x6a/0x84 [1.825951] LR is at irq_get_irq_data+0x7/0x8 [1.830508] pc : [c006465e]lr : [c0063a03]psr: 2133 [1.830512] sp : ee04ff58 ip : fp : [1.842461] r10: r9 : r8 : 0800 [1.847905] r7 : c064e260 r6 : 01dc r5 : 0001 r4 : ee0accc0 [1.854687] r3 : 0002 r2 : 0800 r1 : 01dc r0 : [1.861472] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA Thumb Segment kernel [1.869234] Control: 50c5387d Table: 8000404a DAC: 0015 [1.875215] Process swapper/0 (pid: 1, stack limit = 0xee04e2f8) [1.881463] Stack: (0xee04ff58 to 0xee05) [1.886017] ff40: c061b668 0008 [1.894497] ff60: c0682090 ee0accc0 0003 c001c637 0201 [1.902976] ff80: 0004 c0473820 c0473800 c0459e8d c0680ac0 c000866d 0004 0004 [1.911455] ffa0: ee04ffa8 0004 c047381c 0004 c0473820 c0473800 c0680ac0 0082 [1.919934] ffc0: c0489694 c045265f 0004 0004 c0452135 c000d105 0033 [1.928413] ffe0: c04525b5 c000d111 0033 c000d111 [1.936912] [c006465e] (irq_setup_generic_chip+0x6a/0x84) from [c001c637] (omap_prcm_register_chain_handler+0x147/0x1a0) [1.948516] [c001c637] (omap_prcm_register_chain_handler+0x147/0x1a0) from [c000866d] (do_one_initcall+0x65/0xf4) [1.959500] [c000866d] (do_one_initcall+0x65/0xf4) from [c045265f] (kernel_init+0xab/0x138) [1.968529] [c045265f] (kernel_init+0xab/0x138) from [c000d111] (kernel_thread_exit+0x1/0x6) [1.977632] Code: f7ff f9d1 6b23 1af3 (6043) 086d [1.982684] ---[ end trace 1b75b31a2719ed1c ]--- [1.987526] Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b Cc: Kevin Hilman khil...@ti.com Acked-by: Tero Kristo t-kri...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- v1: -add a bit more description to the changelog as suggested by Tero and Kevin. arch/arm/mach-omap2/prm_common.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/prm_common.c b/arch/arm/mach-omap2/prm_common.c index 873b51d..d28f848 100644 --- a/arch/arm/mach-omap2/prm_common.c +++ b/arch/arm/mach-omap2/prm_common.c @@ -290,7 +290,7 @@ int omap_prcm_register_chain_handler(struct omap_prcm_irq_setup *irq_setup) goto err; } - for (i = 0; i = irq_setup-nr_regs; i++) { + for (i = 0; i irq_setup-nr_regs; i++) { gc = irq_alloc_generic_chip(PRCM, 1, irq_setup-base_irq + i * 32, prm_base, handle_level_irq); -- 1.7.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP3+: fix oops triggered in omap_prcm_register_chain_handler
On Wed, Mar 21, 2012 at 10:13 PM, Kevin Hilman khil...@ti.com wrote: Ming Lei tom.leim...@gmail.com writes: This patch fixes the oos below: As Tero mentioned, please add a bit more description to the changelog about why the iterator is wrong and repost. The revised version has been sent out. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP3+: fix oops triggered in omap_prcm_register_chain_handler
This patch fixes the oos below: [1.790242] Unable to handle kernel NULL pointer dereference at virtual address 0004 [1.798632] pgd = c0004000 [1.801638] [0004] *pgd= [1.805400] Internal error: Oops: 805 [#1] PREEMPT SMP THUMB2 [1.811381] Modules linked in: [1.814601] CPU: 1Not tainted (3.3.0-next-20120320+ #733) [1.820683] PC is at irq_setup_generic_chip+0x6a/0x84 [1.825951] LR is at irq_get_irq_data+0x7/0x8 [1.830508] pc : [c006465e]lr : [c0063a03]psr: 2133 [1.830512] sp : ee04ff58 ip : fp : [1.842461] r10: r9 : r8 : 0800 [1.847905] r7 : c064e260 r6 : 01dc r5 : 0001 r4 : ee0accc0 [1.854687] r3 : 0002 r2 : 0800 r1 : 01dc r0 : [1.861472] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA Thumb Segment kernel [1.869234] Control: 50c5387d Table: 8000404a DAC: 0015 [1.875215] Process swapper/0 (pid: 1, stack limit = 0xee04e2f8) [1.881463] Stack: (0xee04ff58 to 0xee05) [1.886017] ff40: c061b668 0008 [1.894497] ff60: c0682090 ee0accc0 0003 c001c637 0201 [1.902976] ff80: 0004 c0473820 c0473800 c0459e8d c0680ac0 c000866d 0004 0004 [1.911455] ffa0: ee04ffa8 0004 c047381c 0004 c0473820 c0473800 c0680ac0 0082 [1.919934] ffc0: c0489694 c045265f 0004 0004 c0452135 c000d105 0033 [1.928413] ffe0: c04525b5 c000d111 0033 c000d111 [1.936912] [c006465e] (irq_setup_generic_chip+0x6a/0x84) from [c001c637] (omap_prcm_register_chain_handler+0x147/0x1a0) [1.948516] [c001c637] (omap_prcm_register_chain_handler+0x147/0x1a0) from [c000866d] (do_one_initcall+0x65/0xf4) [1.959500] [c000866d] (do_one_initcall+0x65/0xf4) from [c045265f] (kernel_init+0xab/0x138) [1.968529] [c045265f] (kernel_init+0xab/0x138) from [c000d111] (kernel_thread_exit+0x1/0x6) [1.977632] Code: f7ff f9d1 6b23 1af3 (6043) 086d [1.982684] ---[ end trace 1b75b31a2719ed1c ]--- [1.987526] Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b Cc: Tero Kristo t-kri...@ti.com Signed-off-by: Ming Lei tom.leim...@gmail.com --- arch/arm/mach-omap2/prm_common.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/prm_common.c b/arch/arm/mach-omap2/prm_common.c index 873b51d..d28f848 100644 --- a/arch/arm/mach-omap2/prm_common.c +++ b/arch/arm/mach-omap2/prm_common.c @@ -290,7 +290,7 @@ int omap_prcm_register_chain_handler(struct omap_prcm_irq_setup *irq_setup) goto err; } - for (i = 0; i = irq_setup-nr_regs; i++) { + for (i = 0; i irq_setup-nr_regs; i++) { gc = irq_alloc_generic_chip(PRCM, 1, irq_setup-base_irq + i * 32, prm_base, handle_level_irq); -- 1.7.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. How about the 3rd option? 3), take the way in your patch 1) at default, but will switch to register dmtimer directly and bypass 32k-counter if user need it via kernel parameter. As far as I can think of, the situations required for dmtimer are high-frequency perf sample and high precision trace points, so looks it is OK to take 32k-counter at default. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hi, On Tue, Jan 24, 2012 at 7:38 AM, Kevin Hilman khil...@ti.com wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. IMO, under some situations, gp timer clocksource has high precision and is very useful, so hope the Kconfig choice can be kept, or using kernel parameter way, and the patch need to be changed to support the selection. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi Will, On Fri, Jan 27, 2012 at 11:54 PM, Will Deacon will.dea...@arm.com wrote: Ok. Note that on ARM the PMU generates a standard IRQ (i.e. not an NMI) so you may miss samples if they occur during critical kernel sections (and if you look at a profile, spin_unlock_irqrestore will be quite high). Also looks no any irq handler functions can be observed at a profile even though they may be run at a very high frequency. So could we configure ARM PMU interrupt as FIQ to address the above issues? thanks -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible circular locking dependency (3.3-rc2)
On Thu, Feb 16, 2012 at 4:07 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Feb 15, 2012 at 07:54:56PM +0100, Linus Walleij wrote: On Mon, Feb 13, 2012 at 3:53 PM, Ming Lei tom.leim...@gmail.com wrote: Looks 'sysfs_lock' needn't to be held for unregister, so the patch below may fix the problem. Looks correct to me! Acked-by: Linus Walleij linus.wall...@linaro.org Ming, can I have a proper Signed-off-by: line from you so I can apply this patch? Sure, :-) thanks -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible circular locking dependency (3.3-rc2)
@legolas:~# cd /sys/class/gpio/ root@legolas:/sys/class/gpio# echo 2 export root@legolas:/sys/class/gpio# echo 2 unexport root@legolas:/sys/class/gpio# echo 2 export root@legolas:/sys/class/gpio# cd gpio2/ root@legolas:/sys/class/gpio/gpio2# echo 1 value Looks 'sysfs_lock' needn't to be held for unregister, so the patch below may fix the problem. diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 17fdf4b..d773540 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -873,6 +873,7 @@ void gpio_unexport(unsigned gpio) { struct gpio_desc*desc; int status = 0; + struct device *dev = NULL; if (!gpio_is_valid(gpio)) { status = -EINVAL; @@ -884,19 +885,20 @@ void gpio_unexport(unsigned gpio) desc = gpio_desc[gpio]; if (test_bit(FLAG_EXPORT, desc-flags)) { - struct device *dev = NULL; dev = class_find_device(gpio_class, NULL, desc, match_export); if (dev) { gpio_setup_irq(desc, dev, 0); clear_bit(FLAG_EXPORT, desc-flags); - put_device(dev); - device_unregister(dev); } else status = -ENODEV; } mutex_unlock(sysfs_lock); + if (dev) { + device_unregister(dev); + put_device(dev); + } done: if (status) pr_debug(%s: gpio%d status %d\n, __func__, gpio, status); thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm: omap4: allow disabling MPU local timer if 32K timer is enabled
With 32K gp timer, tick can be driven and system can run well, so allow MPU local timer to be disabled if someone requires it, otherwise MPU local timer is always chosen as the default clock_event_device. Signed-off-by: Ming Lei tom.leim...@gmail.com --- arch/arm/mach-omap2/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index d965da4..12cd602 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -46,7 +46,7 @@ config ARCH_OMAP4 select CPU_V7 select ARM_GIC select HAVE_SMP - select LOCAL_TIMERS if SMP + select LOCAL_TIMERS if (SMP !OMAP_32K_TIMER) select PL310_ERRATA_588369 select PL310_ERRATA_727915 select ARM_ERRATA_720789 -- 1.7.9 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: omap4: allow disabling MPU local timer if 32K timer is enabled
On Sat, Feb 11, 2012 at 6:03 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sat, Feb 11, 2012 at 04:31:25PM +0800, Ming Lei wrote: With 32K gp timer, tick can be driven and system can run well, so allow MPU local timer to be disabled if someone requires it, otherwise MPU local timer is always chosen as the default clock_event_device. The point being? IMO, 32K gp timer may be more energy-saving than MPU local timer because gp timer has much less clock frequency and only half interrupts generated in tick mode compared with mpu local timer. What if you want to use NO_HZ? 32K gp timer supports oneshot mode, so it should support NO_HZ. In my test .config, NO_HZ is enabled and system can run well. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: omap4: allow disabling MPU local timer if 32K timer is enabled
On Sat, Feb 11, 2012 at 6:26 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Have you actually checked the tick rate of your timer in /proc/interrupts? Have you checked /proc/timer_list to see what mode the system is in? After checking, the system works at periodic mode, but I still don't know why it can't be in one shot mode. Seems a single clock_event_device can't work well at system with more than one CPU, so the patch doesn't make sense, please ignore it. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Minimum timing resolution in Ubuntu/Linaro on the PandaBoard ES
Hi, On Wed, Feb 8, 2012 at 1:21 PM, Dmitry Antipov dmitry.anti...@linaro.org wrote: On 02/07/2012 02:43 PM, Andrew Richardson wrote: I'm experiencing what appears to be a minimum clock resolution issue in using clock_gettime() on a PandaBoard ES running ubuntu. Do you have CONFIG_OMAP_32K_TIMER enabled in your kernel? Look at 'dmesg | grep clock' and check for the following: ... OMAP clockevent source: GPTIMER1 at 32768 Hz sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms ... Most probably this is the answer - by default, recent OMAPs are configured to use less-accurate, but more energy-saving timer (32KHz) in favor of MPU timer. Sorry, I have a question about the two kind of timers. No matter CONFIG_OMAP_32K_TIMER is defined or not, 'twd' interrupt count is always increased in '/proc/interrupts', and 'gp timer' interrupt count keeps unchanged, so looks MPU timer is still enabled even CONFIG_OMAP_32K_TIMER is disabled, isn't it? After some investigation, I found the change[1] can remove local timer of 'twd', and tick will be driven by 'gp timer' interrupt, but I am not sure if it is the right thing to do. Disable CONFIG_OMAP_32K_TIMER to switch to MPU timer, and check 'dmesg | grep clock' to see: ... OMAP clockevent source: GPTIMER1 at 3840 Hz OMAP clocksource: GPTIMER2 at 3840 Hz sched_clock: 32 bits at 38MHz, resolution 26ns, wraps every 111848ms ... BTW, I have no ideas why clock_getres(CLOCK_REALTIME,...) returns {0, 1} regardless of underlying clock source. I expect {0, 30517} for 32K timer and {0, 26} for MPU timer. [1], 'not select LOCAL_TIMERS for OMAP4 SMP' diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index d965da4..0036218 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -46,7 +46,7 @@ config ARCH_OMAP4 select CPU_V7 select ARM_GIC select HAVE_SMP - select LOCAL_TIMERS if SMP + #select LOCAL_TIMERS if SMP select PL310_ERRATA_588369 select PL310_ERRATA_727915 select ARM_ERRATA_720789 thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Mon, Jan 30, 2012 at 1:36 AM, stephane eranian eran...@googlemail.com wrote: Hi, Ok, so I did a few more tests and there is a serious issue when sampling in frequency mode (the default). I noticed wrong number of samples, so I investigated this some more and instrumented the perf_event kernel code. I found some erratic timer ticks causing broken period adjustments. In fact, the problem is visible using top. I am running a noploop program on CPU0 and nothing else besides top. The noploop program does: for(;;);. That is 100% user. On a 2-way Sometimes it is not 100% user, for example irq/exception handling... system otherwise idle, I expect top to return 50% user 50% idle. Top with the commit: top - 16:19:21 up 5 min, 1 user, load average: 0.23, 0.15, 0.07 Tasks: 70 total, 2 running, 68 sleeping, 0 stopped, 0 zombie Cpu(s): 31.1%us, 2.0%sy, 0.0%ni, 66.2%id, 0.0%wa, 0.0%hi, 0.7%si, 0.0%st That's WRONG Did you reproduce the issue each time or just occasionally? Looks no such issue on my board with 3.3-rc1 plus the 5 extra pmu/emu patches. top - 00:59:15 up 7 min, 1 user, load average: 1.00, 0.73, 0.35 Tasks: 56 total, 2 running, 54 sleeping, 0 stopped, 0 zombie Cpu(s): 42.6%us, 0.2%sy, 0.0%ni, 56.8%id, 0.0%wa, 0.0%hi, 0.4%si, 0.0%st Mem: 1013560k total,50960k used, 962600k free, 6272k buffers Swap:0k total,0k used,0k free,29036k cached PID USER PR NI VIRT RES SHR S %CPU %MEMTIME+ COMMAND 1355 root 20 0 1460 260 216 R 99 0.0 5:07.38 busy 532 root 20 0 000 S0 0.0 0:00.23 kworker/1:1 1356 root 20 0 2552 1120 916 R0 0.1 0:01.93 top Mem: 940292k total, 74984k used, 865308k free, 8020k buffers Swap: 524240k total, 0k used, 524240k free, 37420k cached PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 3770 eranian 20 0 644 160 128 R 99 0.0 0:14.21 noploop 3771 eranian 20 0 2184 1052 804 R 2 0.1 0:00.32 top 1 root 20 0 2564 1528 952 S 0 0.2 0:01.26 init I removed that one liner patch from Ming. The one fiddling with the clockdomains: --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags = CLKDM_CAN_HWSUP, + .flags = CLKDM_CAN_SWSUP, The patch should not affect timer tick logic, and what the patch does is just to revert the commit [1] wrt. emu clock domain. When I rerun, the test, it now work: top - 16:02:51 up 15 min, 1 user, load average: 1.02, 0.46, 0.21 Tasks: 70 total, 2 running, 68 sleeping, 0 stopped, 0 zombie Cpu(s): 47.2%us, 1.0%sy, 0.0%ni, 50.8%id, 0.0%wa, 0.0%hi, 1.0%si, 0.0%st close enough (in it stabilize somehow around 49% which is good) Mem: 940292k total, 75288k used, 865004k free, 8004k buffers Swap: 524240k total, 0k used, 524240k free, 37408k cached PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 3771 eranian 20 0 644 160 128 R 100 0.0 0:34.44 noploop Although the patch fixes PMU interrupts, it breaks the timer tick logic somehow. The perf problem is related to timer tick. I am hoping that the tradeoff is not: PMU interrupts but broken timer ticks vs. No PMU interrupts but working timer ticks [1], 3c50729b3fa1cd8ca1f347e6caf1081204cf1a7c ARM: OMAP4: PM: Initialise all the clockdomains to supported states thanks -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
On Mon, Jan 30, 2012 at 9:43 PM, stephane eranian eran...@googlemail.com wrote: Same results for me with 3.3.0-rc1 + 5 patches. In fact, I think the only effect of the patch is to enable pmu interrupt handling, which may cause so much difference? Also maybe you should put 'noploop' to run on CPU1 and you may observe a more accurate result of 'top'. On ARM, almost handling of all IRQs from gic is run on CPU0 at default, which may cause your issue. top - 14:42:34 up 8 min, 1 user, load average: 0.70, 0.29, 0.15 Tasks: 75 total, 2 running, 73 sleeping, 0 stopped, 0 zombie Cpu(s): 32.9%us, 1.3%sy, 0.0%ni, 65.8%id, 0.0%wa, 0.0%hi, 0.0%si, 0.0%st Mem: 940232k total, 118520k used, 821712k free, 8080k buffers Swap: 524240k total, 0k used, 524240k free, 79432k cached PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 3868 eranian 20 0 644 160 128 R 99 0.0 0:53.34 noploop 3870 eranian 20 0 2284 1060 804 R 3 0.1 0:00.63 top 1 root 20 0 2564 1532 952 S 0 0.2 0:01.26 init I am connecting to the board via ssh. But the results don't look correct to me. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Fri, Jan 27, 2012 at 8:13 PM, Will Deacon will.dea...@arm.com wrote: There must be something I am missing here. Did this lead anywhere in the end? It seems as though Ming Lei has a working setup but Stephane is unable to replicate it, despite applying the necessary patches and trying an updated bootloader. In fact, I don't think stephane has tried the patch in the link[1] and reported the test result, even though I have asked him to do it for several times. Drastic suggestion: Stephane, could you try a kernel *binary* from Ming Lei? If that works then you're probably just missing a patch. If it doesn't, then there must be something different between your boards. [1], http://marc.info/?l=linux-arm-kernelm=132697975416659w=2 thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, 2012/1/27 Will Deacon will.dea...@arm.com: Mans, On Fri, Jan 27, 2012 at 12:56:35PM +, Måns Rullgård wrote: Will Deacon will.dea...@arm.com writes: Did this lead anywhere in the end? It seems as though Ming Lei has a working setup but Stephane is unable to replicate it, despite applying the necessary patches and trying an updated bootloader. With the patches listed above plus the one in [1], I get PMU interrupts. However, unless I restrict the profiled process to one CPU (taskset 1 perf record ...), I get a panic in armpmu_event_update() with the 'event' argument being null when called from armv7pmu_handle_irq(). [1] http://article.gmane.org/gmane.linux.ports.arm.omap/69696 Great, thanks for trying this out. Which version of the kernel were you using? The patch is required for 3.3-rc1 in case that omap4 pmu works well. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
On Fri, Jan 20, 2012 at 9:47 PM, stephane eranian eran...@googlemail.com wrote: Started afresh from: 90a4c0f uml: fix compile for x86-64 And added 3, 4, 5, 6: 603c316 arm: omap4: pmu: support runtime pm 4899fbd arm: omap4: support pmu d737bb1 arm: omap4: create pmu device via hwmod 4e0259e arm: omap4: hwmod: introduce emu hwmod Still no interrupts firing. I am using your .config file. My HW: CPU implementer : 0x41 CPU architecture: 7 CPU variant : 0x1 CPU part : 0xc09 CPU revision : 2 Hardware : OMAP4 Panda board Revision : 0020 There must be something I am missing here. Have you applied the patch in link[1]? thanks, -- Ming Lei [1], http://marc.info/?l=linux-arm-kernelm=132697975416659w=2 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian eran...@googlemail.com wrote: Hi, Ok some update on this. With your .config file + 3.2.0 (Linus) + patch 3, 4, 5, 6, I get a kernel that You forget patch 1 and patch 2? boots. It does recognize the PMU. However, it still does not count correctly and I believe for the same reason.: no interrupts are delivered. I run a cycle burner program on CPU0, I watch /proc/interrupts. and then I run libpfm4 program that does per-cpu monitoring on CPU0 and print the counts every second: I just run 'perf top', then watch output of '/proc/interrupts' in another terminal. I am sure I can see perf is OK and interrupts are generated on my pandaboard. $ sudo ./syst_count -d 10 -p -c 0 -e cpu_cycles press CTRL-C to quit before 10s time limit # 1s - CPU0 G0 1008129147 cpu_cycles (scaling 0.00%, ena=1000152588, run=1000152588) # 2s - CPU0 G0 2016240766 cpu_cycles (scaling 0.00%, ena=2000335693, run=2000335693) # 3s - CPU0 G0 3024249265 cpu_cycles (scaling 0.00%, ena=3000427245, run=3000427245) # 4s - CPU0 G0 4072779364 cpu_cycles (scaling 0.00%, ena=4040710449, run=4040710449) # 5s - CPU0 G0 785954705 cpu_cycles (scaling 0.00%, ena=5040954589, run=5040954589) # 6s - CPU0 G0 1803397848 cpu_cycles (scaling 0.00%, ena=6050384520, run=6050384520) # 7s - You clearly see that after 4s you've reached the 32-bit limit of the counter and then you wrap around. It should show 5 billions or so cycles. Over the entire run, no arm-pmu interrupt was delivered according to /proc/interrupts. I guess you can test the same condition using perf directly, use a program that burns cycles for a know duration. Try 4s and then 4s. I use 1s vs. 10s and I expect the count to be 10x larger in the latter test case. If it's not then, interrupts are not coming in, On Thu, Jan 19, 2012 at 2:21 AM, Ming Lei ming@canonical.com wrote: Hi, On Thu, Jan 19, 2012 at 5:58 AM, stephane eranian eran...@googlemail.com wrote: Ming, Ok, so I used Linus' tree @ It already includes patches #1 and #2. I applied 4-6. The patch #3 is missed? Recompiled but my kernel does not boot, I don't see anything on the serial console. Could be a broken I don't think that the patches can cause your non boot, you can try the linus tree kernel first, then try the patches. .config file. Could you send me your .config for Panda? See the attachment. Thanks. On Wed, Jan 18, 2012 at 11:07 AM, Ming Lei ming@canonical.com wrote: Hi, On Wed, Jan 18, 2012 at 5:54 PM, stephane eranian eran...@googlemail.com Should I use Will's -next tree as the base instead of Linus'? Either one is OK. If you use linus tree as base, you need to apply the #1 and #2 patch manually. Given that MARC is shutdown today, would you mind packing those patches into a tarball and sending them to me directly? See attachment, which includes the patches from #3 to #6. When you mention Will's -next tree, are you talking about: git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-next/perf It is perf/omap4 brach, you can pick up the two patches[1][2] directly from the branch. thanks, -- Ming Lei [1], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=7924a3eba0766348d6d6a56cbb9873cdbcab0d8c [2], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=bde071f005e2dc71378aff69e86b961d8cd7922f -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian eran...@googlemail.com wrote: On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei ming@canonical.com wrote: Hi, On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian eran...@googlemail.com wrote: Hi, Ok some update on this. With your .config file + 3.2.0 (Linus) + patch 3, 4, 5, 6, I get a kernel that You forget patch 1 and patch 2? They are already in 3.2.0. Unless I am mistaken. Sorry, I just found that they have been merged to 3.2. are you sure you don't have anything else applied? Yes, I am sure, maybe it is caused by uboot and mlo, I will email them to you in private mail. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Thu, Jan 19, 2012 at 9:14 PM, Ming Lei ming@canonical.com wrote: On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian eran...@googlemail.com wrote: On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei ming@canonical.com wrote: Hi, On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian eran...@googlemail.com wrote: Hi, Ok some update on this. With your .config file + 3.2.0 (Linus) + patch 3, 4, 5, 6, I get a kernel that You forget patch 1 and patch 2? They are already in 3.2.0. Unless I am mistaken. Sorry, I just found that they have been merged to 3.2. After a double check, the two patches are not merged to 3.2, but have been merged to the latest linus tree and can be seen in 3.3-rc1. Also the commit 3c50729b(ARM: OMAP4: PM: Initialise all the clockdomains to supported states) has been merged to linus tree too. So if you just tested the latest linus tree simply, you need to apply the patch[1] (I have mentioned the problem in the thread.) thanks, -- Ming Lei [1], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..41d2260 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags= CLKDM_CAN_HWSUP, + .flags= CLKDM_CAN_SWSUP, }; static struct clockdomain l3_dma_44xx_clkdm = { -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Thu, Jan 19, 2012 at 9:32 PM, stephane eranian eran...@googlemail.com wrote: On Thu, Jan 19, 2012 at 2:26 PM, Ming Lei ming@canonical.com wrote: Hi, On Thu, Jan 19, 2012 at 9:14 PM, Ming Lei ming@canonical.com wrote: On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian eran...@googlemail.com wrote: On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei ming@canonical.com wrote: Hi, On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian eran...@googlemail.com wrote: Hi, Ok some update on this. With your .config file + 3.2.0 (Linus) + patch 3, 4, 5, 6, I get a kernel that You forget patch 1 and patch 2? They are already in 3.2.0. Unless I am mistaken. Sorry, I just found that they have been merged to 3.2. After a double check, the two patches are not merged to 3.2, but have been merged to the latest linus tree and can be seen in 3.3-rc1. Also the commit 3c50729b(ARM: OMAP4: PM: Initialise all the clockdomains to supported states) has been merged to linus tree too. So if you just tested the latest linus tree simply, you need to apply the patch[1] (I have mentioned the problem in the thread.) Changing LMO, u-boot.bin did not help. Even with perf top I get no interrupts. My Linus tree is at commit fa1952b: [6] 11891e1 arm: omap4: pmu: support runtime pm [5] 25fab8a arm: omap4: support pmu [4] fddef77 arm: omap4: create pmu device via hwmod fa1952b ARM: OMAP4: hwmod data: Add support for the debug modules Sorry, there is no commit fa1952b in linus[1] tree at all, so you are not testing linus tree... If you'd like to follow my instructions, I can help you further. ccb19d2 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net c3b5003 tg3: Fix single-vector MSI-X code I think [1] had conflicts when applying it to the tree. It is only one line(one character) change, you can do it manually. thanks, -- Ming Lei [1], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..41d2260 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags = CLKDM_CAN_HWSUP, + .flags = CLKDM_CAN_SWSUP, }; [1], http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=summary thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi Will and stephane, On Wed, Jan 18, 2012 at 12:18 PM, Ming Lei ming@canonical.com wrote: Hi stephane Will, On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian eran...@googlemail.com wrote: See the dmesg from my 3.2 kernel: [ 0.00] Booting Linux on physical CPU 0[ 0.00] But if you test omap4 perf against -next kernel, pmu won't work because the commit[1] may put 'emu_sys_clkdm' clock domain into HW_AUTO mode, so writing pmu register may not take effect. I have found the similar problem on cam clock domain before[2]. CD_EMU is very simliar with CD_CAM in the point below: CD_EMU has no static or module wake-up dependency with any other clock domain of the device.[3] So the patch[4] can make omap4 pmu work on -next tree. Shilimkar, care to comment on the patch[4]? thanks, -- Ming Lei [1], commit 3c50729b3fa1cd8ca1f347e6caf1081204cf1a7c Author: Santosh Shilimkar santosh.shilim...@ti.com Date: Wed Jan 5 22:03:17 2011 +0530 ARM: OMAP4: PM: Initialise all the clockdomains to supported states Initialise hardware supervised mode for all clockdomains if it's supported. Initiate sleep transition for other clockdomains, if they are not being used. [2], http://www.spinics.net/lists/linux-omap/msg61911.html [3], 3.6.12.3 of OMAP4 TRM [4], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..41d2260 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags= CLKDM_CAN_HWSUP, + .flags= CLKDM_CAN_SWSUP, }; static struct clockdomain l3_dma_44xx_clkdm = { -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Wed, Jan 18, 2012 at 5:54 PM, stephane eranian eran...@googlemail.com Should I use Will's -next tree as the base instead of Linus'? Either one is OK. If you use linus tree as base, you need to apply the #1 and #2 patch manually. Given that MARC is shutdown today, would you mind packing those patches into a tarball and sending them to me directly? See attachment, which includes the patches from #3 to #6. When you mention Will's -next tree, are you talking about: git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-next/perf It is perf/omap4 brach, you can pick up the two patches[1][2] directly from the branch. thanks, -- Ming Lei [1], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=7924a3eba0766348d6d6a56cbb9873cdbcab0d8c [2], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=bde071f005e2dc71378aff69e86b961d8cd7922f omap4_pmu.tar.gz Description: GNU Zip compressed data
Re: oprofile and ARM A9 hardware counter
On Wed, Jan 18, 2012 at 7:39 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Wed, Jan 18, 2012 at 10:33 AM, Ming Lei ming@canonical.com wrote: Hi Will and stephane, On Wed, Jan 18, 2012 at 12:18 PM, Ming Lei ming@canonical.com wrote: Hi stephane Will, On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian eran...@googlemail.com wrote: See the dmesg from my 3.2 kernel: [ 0.00] Booting Linux on physical CPU 0[ 0.00] But if you test omap4 perf against -next kernel, pmu won't work because the commit[1] may put 'emu_sys_clkdm' clock domain into HW_AUTO mode, so writing pmu register may not take effect. I have found the similar problem on cam clock domain before[2]. CD_EMU is very simliar with CD_CAM in the point below: CD_EMU has no static or module wake-up dependency with any other clock domain of the device.[3] So the patch[4] can make omap4 pmu work on -next tree. Shilimkar, care to comment on the patch[4]? thanks, -- Ming Lei [1], commit 3c50729b3fa1cd8ca1f347e6caf1081204cf1a7c Author: Santosh Shilimkar santosh.shilim...@ti.com Date: Wed Jan 5 22:03:17 2011 +0530 ARM: OMAP4: PM: Initialise all the clockdomains to supported states Initialise hardware supervised mode for all clockdomains if it's supported. Initiate sleep transition for other clockdomains, if they are not being used. [2], http://www.spinics.net/lists/linux-omap/msg61911.html [3], 3.6.12.3 of OMAP4 TRM [4], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..41d2260 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -390,7 +390,7 @@ static struct clockdomain emu_sys_44xx_clkdm = { .prcm_partition = OMAP4430_PRM_PARTITION, .cm_inst = OMAP4430_PRM_EMU_CM_INST, .clkdm_offs = OMAP4430_PRM_EMU_CM_EMU_CDOFFS, - .flags = CLKDM_CAN_HWSUP, + .flags = CLKDM_CAN_SWSUP, }; NAK. You don't need this patch. What you saw on CAMERA was indeed a known bug but emulation domain has no such issues. So the accesses to emulation register should continue to work with the clock-domain being kept under hardware supervision. But why can this patch make omap4 pmu work? Without the patch, there are no CTI interrupts generated for pmu irq. thanks -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Thu, Jan 19, 2012 at 5:58 AM, stephane eranian eran...@googlemail.com wrote: Ming, Ok, so I used Linus' tree @ It already includes patches #1 and #2. I applied 4-6. The patch #3 is missed? Recompiled but my kernel does not boot, I don't see anything on the serial console. Could be a broken I don't think that the patches can cause your non boot, you can try the linus tree kernel first, then try the patches. .config file. Could you send me your .config for Panda? See the attachment. Thanks. On Wed, Jan 18, 2012 at 11:07 AM, Ming Lei ming@canonical.com wrote: Hi, On Wed, Jan 18, 2012 at 5:54 PM, stephane eranian eran...@googlemail.com Should I use Will's -next tree as the base instead of Linus'? Either one is OK. If you use linus tree as base, you need to apply the #1 and #2 patch manually. Given that MARC is shutdown today, would you mind packing those patches into a tarball and sending them to me directly? See attachment, which includes the patches from #3 to #6. When you mention Will's -next tree, are you talking about: git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-next/perf It is perf/omap4 brach, you can pick up the two patches[1][2] directly from the branch. thanks, -- Ming Lei [1], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=7924a3eba0766348d6d6a56cbb9873cdbcab0d8c [2], http://git.kernel.org/?p=linux/kernel/git/will/linux.git;a=commit;h=bde071f005e2dc71378aff69e86b961d8cd7922f -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html conf.tar.gz Description: GNU Zip compressed data
Re: oprofile and ARM A9 hardware counter
Hi stephane Will, On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian eran...@googlemail.com wrote: See the dmesg from my 3.2 kernel: [ 0.00] Booting Linux on physical CPU 0[ 0.00] Looks no obvious failure can be found from your 'dmesg'. I have run upstream 3.2 kernel plus 6 omap4 pmu patches below and found perf can work well on my panda board. 0001-arm-introduce-cross-trigger-interface-helpers.patch 0002-arm-pmu-allow-platform-specific-irq-enable-disable-h.patch 0003-arm-omap4-hwmod-introduce-emu-hwmod.patch or Benoit's debugss patch[2] 0004-arm-omap4-create-pmu-device-via-hwmod.patch[3] 0005-arm-omap4-support-pmu.patch[4] 0006-arm-omap4-pmu-support-runtime-pm.patch[5] Could you verify the above patches on 3.2 to see if perf can work well? If it doesn't, I may share my u-boot and mlo for your test if you'd like to do. BTW: #1 and #2 have been in Will's -next tree. thanks, -- Ming Lei [1], uname -a cat /proc/interrupts [root@root]#uname -a Linux beagleboard 3.2.0+ #480 SMP PREEMPT Wed Jan 18 11:38:33 CST 2012 armv7l GNU/Linux [root@root]#cat /proc/interrupts CPU0 CPU1 29: 29014 17353 GIC twd 33: 56231 0 GIC arm-pmu 34: 0 25778 GIC arm-pmu [2], http://marc.info/?l=linux-omapm=132162118104901w=2 [3],http://marc.info/?t=13222762152r=1w=2 [4],http://marc.info/?t=13222762172r=1w=2 [5],http://marc.info/?t=13222762173r=1w=2 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: oprofile and ARM A9 hardware counter
Hi, On Tue, Jan 10, 2012 at 6:49 AM, Will Deacon will.dea...@arm.com wrote: On Mon, Jan 09, 2012 at 04:39:09PM +, Maynard Johnson wrote: On 01/08/2012 8:58 PM, Lik Lik wrote: Hi all, Hi guys [adding a bunch of people to CC because this issue is really annoying me now], I am an oprofile user and I need to profile one of my applications on a TI OMAP4 platform (pandaboard, to be specific). I have ubuntu 11.10 installed. My problem is that oprofile always use the timer interrupt mode but doesn't recognize the hardware counters, which I am sure my platform has. First and foremost, we can't currently generate PMU interrupts on OMAP4 in mainline. There are some additional patches required for these to work: http://marc.info/?l=linux-arm-kernelm=131946761428296w=2 However, as Stephane has pointed out here: http://marc.info/?l=linux-omapm=132585784125352w=2 the interrupts still don't seem to work, even with the patches above applied. Ming Lei doesn't seem to be replying to email anymore, so maybe somebody Sorry, I am on a trip now and no pandboard at my hand, so I may have time to verify the latest mainline next week after I return home. I remembered that last time I verified these patches on 3.2-rc2 and 3.2-rc2 next tree, and perf did work well on my pandaboard. Also seems there were reports that omap4 perf may not work on some specific uboot version even with these patches. else on linux-omap could please help us? I'm hoping that we're just missing some patches from somewhere, but it would be great if somebody could verify this (and indeed, verify that the interrupts we're registering in the thread above look sane). OProfile userspace support for ARM Cortex-A9 was added by Will Deacon in June 2010. This support is available in OProfile 0.9.7. According to Will's posting, the kernel support was due to be added to Ubuntu Maverick, so I would expect your version should support CA9 out of the box. If not already using oprofile 0.9.7, please upgrade to that version and retry. If it still doesn't work, please re-post with complete information (kernel version, oprofile command output, and contents of /dev/oprofile/cpu_type). If with the latest OProfile, `timer mode' is still reported then you should check that you have CONFIG_HW_PERF_EVENTS enabled in your kernel. It still won't work though, because of the problems I mentioned above. If debug message from 'dmesg' can be provided, maybe we can find clue about the problem. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 7/8] media: video: introduce object detection driver module
Hi Sylwester, Thanks for your review. On Fri, Dec 30, 2011 at 1:16 AM, Sylwester Nawrocki snj...@gmail.com wrote: Hi Ming, On 12/14/2011 03:00 PM, Ming Lei wrote: This patch introduces object detection generic driver. The driver is responsible for all v4l2 stuff, buffer management and other general things, and doesn't touch object detection hardware directly. Several interfaces are exported to low level drivers (such as the coming omap4 FD driver) which will communicate with object detection hw module. So the driver will make driving object detection hw modules more easy. TODO: - implement object detection setting interfaces with v4l2 controls or ext controls Signed-off-by: Ming Lei ming@canonical.com --- v2: - extend face detection driver to object detection driver - introduce subdevice and media entity - provide support to detect object from media HW --- drivers/media/video/Kconfig | 2 + drivers/media/video/Makefile | 1 + drivers/media/video/odif/Kconfig | 7 + drivers/media/video/odif/Makefile | 1 + drivers/media/video/odif/odif.c | 890 + drivers/media/video/odif/odif.h | 157 +++ 6 files changed, 1058 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/odif/Kconfig create mode 100644 drivers/media/video/odif/Makefile create mode 100644 drivers/media/video/odif/odif.c create mode 100644 drivers/media/video/odif/odif.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 5684a00..8740ee9 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -1166,3 +1166,5 @@ config VIDEO_SAMSUNG_S5P_MFC MFC 5.1 driver for V4L2. endif # V4L_MEM2MEM_DRIVERS + +source drivers/media/video/odif/Kconfig diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index bc797f2..259c8d8 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -197,6 +197,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o obj-y += davinci/ obj-$(CONFIG_ARCH_OMAP) += omap/ +obj-$(CONFIG_ODIF) += odif/ ccflags-y += -Idrivers/media/dvb/dvb-core ccflags-y += -Idrivers/media/dvb/frontends diff --git a/drivers/media/video/odif/Kconfig b/drivers/media/video/odif/Kconfig new file mode 100644 index 000..5090bd6 --- /dev/null +++ b/drivers/media/video/odif/Kconfig @@ -0,0 +1,7 @@ +config ODIF + depends on VIDEO_DEV VIDEO_V4L2 + select VIDEOBUF2_PAGE + tristate Object Detection module + help + The ODIF is a object detection module, which can be integrated into + some SoCs to detect objects in images or video. diff --git a/drivers/media/video/odif/Makefile b/drivers/media/video/odif/Makefile new file mode 100644 index 000..a55ff66 --- /dev/null +++ b/drivers/media/video/odif/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_ODIF) += odif.o diff --git a/drivers/media/video/odif/odif.c b/drivers/media/video/odif/odif.c new file mode 100644 index 000..381ab9d --- /dev/null +++ b/drivers/media/video/odif/odif.c @@ -0,0 +1,890 @@ +/* + * odif.c -- object detection module driver + * + * Copyright (C) 2011 Ming Lei (ming@canonical.com) + * + * This file is based on drivers/media/video/vivi.c. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +/*/ + +#include linux/module.h +#include linux/fs.h +#include linux/mm.h +#include linux/signal.h +#include linux/wait.h +#include linux/poll.h +#include linux/mman.h +#include linux/pm_runtime.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/interrupt.h +#include asm/uaccess.h +#include asm/byteorder.h +#include asm/io.h +#include odif.h + +#define input_from_user(dev) \ + (dev-input == OD_INPUT_FROM_USER_SPACE) + +#define DEFAULT_PENDING_RESULT_CNT 8 + +static unsigned debug = 0; +module_param(debug, uint, 0644); +MODULE_PARM_DESC(debug, activates debug info); + +static unsigned result_cnt_threshold = DEFAULT_PENDING_RESULT_CNT; +module_param
Re: [QUESTION] How to set static/dynamic dependency between clock domains
Hi Shilimkar, Sorry for the delay. On Mon, Dec 19, 2011 at 2:17 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: The patches are fine but I am surprised that 'dyndep.patch' is helping you. L3_2 dependency is always enabled with ISS or Camera and it should have worked. clkdm_add_wkdep(iss_clkdm, l3_2_clkdm), shoudn't do anything since the bit is read-only. Yes, you are right. After some investigating, I found that clkdm_add_wkdep(iss_clkdm, l3_2_clkdm) returns failure, so clkdms_setup is bypassed and the issue is avoided. Can you check value of CM_CAM_STATICDEP [ 0x4A009004], The value is 0x40 after and before the patch. after your patch. I am suspecting that for some reason l3_1 dep. is getting enabled which might be helping your case. Seems no changes after adding clkdm_add_wkdep(iss_clkdm, l3_1_clkdm) on the problem. The only change on iss(CAM) clock domain setting in your commit[1] is to configure CLKTRCTRL as HW_AUTO, instead of previous SW_WKUP. Once I change flags of iss to CLKDM_CAN_SWSUP [2], the issue can be fixed, so I am wondering if something is wrong about HW_AUTO mode of CAM clock domain. -- Ming Lei [1], ARM: OMAP4: PM: Initialise all the clockdomains to supported states [2], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..1dfc7ce 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -359,7 +359,7 @@ static struct clockdomain iss_44xx_clkdm = { .clkdm_offs = OMAP4430_CM2_CAM_CAM_CDOFFS, .wkdep_srcs = iss_wkup_sleep_deps, .sleepdep_srcs= iss_wkup_sleep_deps, - .flags= CLKDM_CAN_HWSUP_SWSUP, + .flags= CLKDM_CAN_SWSUP, }; -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [QUESTION] How to set static/dynamic dependency between clock domains
On Fri, Dec 23, 2011 at 4:45 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Fri, Dec 23, 2011 at 2:00 PM, Ming Lei tom.leim...@gmail.com wrote: Hi Shilimkar, Sorry for the delay. On Mon, Dec 19, 2011 at 2:17 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: The patches are fine but I am surprised that 'dyndep.patch' is helping you. L3_2 dependency is always enabled with ISS or Camera and it should have worked. clkdm_add_wkdep(iss_clkdm, l3_2_clkdm), shoudn't do anything since the bit is read-only. Yes, you are right. After some investigating, I found that clkdm_add_wkdep(iss_clkdm, l3_2_clkdm) returns failure, so clkdms_setup is bypassed and the issue is avoided. Good. Can you check value of CM_CAM_STATICDEP [ 0x4A009004], The value is 0x40 after and before the patch. after your patch. I am suspecting that for some reason l3_1 dep. is getting enabled which might be helping your case. Seems no changes after adding clkdm_add_wkdep(iss_clkdm, l3_1_clkdm) on the problem. The only change on iss(CAM) clock domain setting in your commit[1] is to configure CLKTRCTRL as HW_AUTO, instead of previous SW_WKUP. Once I change flags of iss to CLKDM_CAN_SWSUP [2], the issue can be fixed, so I am wondering if something is wrong about HW_AUTO mode of CAM clock domain. Now I recollect the issue and also track a patch in the internal product tree. Same is attached and also in the end of the email. I am lopping Miguel who wrote the patch and Benoit who acked it. This should sort out your issue as you have already verified it works. Yes, I am sure. Regards Santosh From 972d7bb544d3197d7fc1a6c6eb0e2c9cc08d5e9d Mon Sep 17 00:00:00 2001 From: Miguel Vadillo vadi...@ti.com Date: Tue, 21 Jun 2011 09:59:45 -0500 Subject: [PATCH 1/2] OMAP: clockdomain: set iss clk domain to just SWSUP Since CAM domain(ISS) has no module wake-up dependency with any other clock domain of the device and the dynamic dependency from L3_main_2 is always disabled, the domain needs to be in force wakeup in order to be able to access it for configure(sysconfig) it or use it. Also since there is no clock in the domain managed automatically by the hardware, there is no use to configure automatic clock domain transition. SW should keep the SW_WKUP domain transition as long as a module in the domain is required to be functional. Signed-off-by: Miguel Vadillo vadi...@ti.com Acked-by: Benoit Coussonb-cous...@ti.com Please feel free to add: Reported-and-tested-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/clockdomains44xx_data.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 6ac8fe2..8d1a061 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -605,7 +605,7 @@ static struct clockdomain iss_44xx_clkdm = { .clkdm_offs = OMAP4430_CM2_CAM_CAM_CDOFFS, .wkdep_srcs = iss_wkup_sleep_deps, .sleepdep_srcs = iss_wkup_sleep_deps, - .flags = CLKDM_CAN_HWSUP_SWSUP, + .flags = CLKDM_CAN_SWSUP, .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP44XX), }; -- 1.7.4.1 thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[QUESTION] How to set static/dynamic dependency between clock domains
Hi, I found that face detection module(fdif) can't be enabled successfully without the change in [1]. Looks like it is not a good way to do it because the dynamic dependency of CD_CAM on l3_2 is disabled at default, not like other ones, so I guess that it may work if the dynamic dependency is enabled by setting bit L3_2_DYNDEP to CM_CAM_DYNAMICDEP. After greping over arch/arm/mach-omap2 and arch/arm/plat-omap, I did not find any operations on CM_*_DYNAMICDEP register, so I am wondering if dynamic dependency is not used on omap4 at all now. Also I have tried to add 'l3_2_clkdm' into dependency table of iss_clkdm in [2], and it doesn't work. How could I cope with the clock domain dependency so that fdif can be enabled successfully? thanks,-- Ming Lei [1], add static dependency diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.cindex c264ef7..23e1f8c 100644--- a/arch/arm/mach-omap2/pm44xx.c+++ b/arch/arm/mach-omap2/pm44xx.c@@ -198,6 +198,7 @@ static int __init omap4_pm_init(void) int ret; struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm; struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;+ struct clockdomain *iss_clkdm; if (!cpu_is_omap44xx()) return -ENODEV;@@ -227,8 +228,10 @@ static int __init omap4_pm_init(void) l3_2_clkdm = clkdm_lookup(l3_2_clkdm); l4_per_clkdm = clkdm_lookup(l4_per_clkdm); ducati_clkdm = clkdm_lookup(ducati_clkdm);+ iss_clkdm = clkdm_lookup(iss_clkdm); if ((!mpuss_clkdm) || (!emif_clkdm) || (!l3_1_clkdm) ||- (!l3_2_clkdm) || (!ducati_clkdm) || (!l4_per_clkdm))+ (!l3_2_clkdm) || (!ducati_clkdm) || (!l4_per_clkdm) ||+ (!iss_clkdm)) goto err2; ret = clkdm_add_wkdep(mpuss_clkdm, emif_clkdm);@@ -237,6 +240,7 @@ static int __init omap4_pm_init(void) ret |= clkdm_add_wkdep(mpuss_clkdm, l4_per_clkdm); ret |= clkdm_add_wkdep(ducati_clkdm, l3_1_clkdm); ret |= clkdm_add_wkdep(ducati_clkdm, l3_2_clkdm);+ ret |= clkdm_add_wkdep(iss_clkdm, l3_2_clkdm); if (ret) { pr_err(Failed to add MPUSS - L3/EMIF/L4PER, DUCATI - L3 wakeup dependency\n); [2], diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c b/arch/arm/mach-omap2/clockdomains44xx_data.c index 9299ac2..3f8f6a9 100644 --- a/arch/arm/mach-omap2/clockdomains44xx_data.c +++ b/arch/arm/mach-omap2/clockdomains44xx_data.c @@ -65,6 +65,7 @@ static struct clkdm_dep ducati_wkup_sleep_deps[] = { static struct clkdm_dep iss_wkup_sleep_deps[] = { { .clkdm_name = ivahd_clkdm }, { .clkdm_name = l3_1_clkdm }, + { .clkdm_name = l3_2_clkdm }, { .clkdm_name = l3_emif_clkdm }, { NULL }, }; -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [QUESTION] How to set static/dynamic dependency between clock domains
Hi, On Fri, Dec 16, 2011 at 9:24 PM, Ming Lei tom.leim...@gmail.com wrote: [1], add static dependency Sorry for the mess, see attachment for the change. thanks, -- Ming Lei diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c index c264ef7..23e1f8c 100644 --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm44xx.c @@ -198,6 +198,7 @@ static int __init omap4_pm_init(void) int ret; struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm; struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm; + struct clockdomain *iss_clkdm; if (!cpu_is_omap44xx()) return -ENODEV; @@ -227,8 +228,10 @@ static int __init omap4_pm_init(void) l3_2_clkdm = clkdm_lookup(l3_2_clkdm); l4_per_clkdm = clkdm_lookup(l4_per_clkdm); ducati_clkdm = clkdm_lookup(ducati_clkdm); + iss_clkdm = clkdm_lookup(iss_clkdm); if ((!mpuss_clkdm) || (!emif_clkdm) || (!l3_1_clkdm) || - (!l3_2_clkdm) || (!ducati_clkdm) || (!l4_per_clkdm)) + (!l3_2_clkdm) || (!ducati_clkdm) || (!l4_per_clkdm) || + (!iss_clkdm)) goto err2; ret = clkdm_add_wkdep(mpuss_clkdm, emif_clkdm); @@ -237,6 +240,7 @@ static int __init omap4_pm_init(void) ret |= clkdm_add_wkdep(mpuss_clkdm, l4_per_clkdm); ret |= clkdm_add_wkdep(ducati_clkdm, l3_1_clkdm); ret |= clkdm_add_wkdep(ducati_clkdm, l3_2_clkdm); + ret |= clkdm_add_wkdep(iss_clkdm, l3_2_clkdm); if (ret) { pr_err(Failed to add MPUSS - L3/EMIF/L4PER, DUCATI - L3 wakeup dependency\n);
Re: [QUESTION] How to set static/dynamic dependency between clock domains
On Fri, Dec 16, 2011 at 11:13 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Fri, Dec 16, 2011 at 7:20 PM, Ming Lei tom.leim...@gmail.com wrote: Hi, On Fri, Dec 16, 2011 at 9:24 PM, Ming Lei tom.leim...@gmail.com wrote: [1], add static dependency Sorry for the mess, see attachment for the change. Your patch is setting static dependency between ISS and l3_2. It's not dynamic dep. Dynamic dep is managed through hardware and they Yes, I see now, L3_2_DYNDEP of CM_CAM_DYNAMICDEP is read only. are not configurable. But with static dep. you can over-ride that behavior as you have done. This will impact power since l3_2 can't idle as long as iss clock domain is active. I need to check internal code-base but I don't remember this issue observed so far. In fact, I saw the issue on 3.2.0-rc5-next-20111216, and even no such issue on 3.2.0-rc5. After some bisecting, I found below is the first commit on which the issue can be observed: commit 3c50729b3fa1cd8ca1f347e6caf1081204cf1a7c Author: Santosh Shilimkar santosh.shilim...@ti.com Date: Wed Jan 5 22:03:17 2011 +0530 ARM: OMAP4: PM: Initialise all the clockdomains to supported states Initialise hardware supervised mode for all clockdomains if it's supported. Initiate sleep transition for other clockdomains, if they are not being used. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [QUESTION] How to set static/dynamic dependency between clock domains
Hi, On Fri, Dec 16, 2011 at 9:24 PM, Ming Lei tom.leim...@gmail.com wrote: Also I have tried to add 'l3_2_clkdm' into dependency table of iss_clkdm in [2], and it doesn't work. Also, I still have the question why the static dependency isn't generated from the table .wkdep_srcs of struct clockdomain? Seems the static dep info has been put inside the tables. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [QUESTION] How to set static/dynamic dependency between clock domains
On Sat, Dec 17, 2011 at 12:09 AM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: No. The table reflects the actual possible combination on hardware. You can't add anything there arbitrary if that combination is not supported. But that makes me wonder how the patch worked. I will look at this in detail next week. Thanks, I use the below hwmod data patch[1] and omap device patch[2] to bring up fdif module, and Benoit has ACKed on patch [1]. [1], http://marc.info/?l=linux-kernelm=132387140703874w=2 [2], http://marc.info/?l=linux-kernelm=132387155403929w=2 thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 0/7] media: introduce object detection(OD) driver
Hi, These v2 patches(against -next tree) introduce v4l2 based object detection(OD) device driver, and enable face detection hardware[1] on omap4 SoC.. The idea of implementing it on v4l2 is from from Alan Cox, Sylwester and Greg-Kh. For verification purpose, I write one user space utility[2] to test the module and driver, follows its basic functions: - detect faces in input grayscal picture(PGM raw, 320 by 240) - detect faces in input y8 format video stream - plot a rectangle to mark the detected faces, and save it as another same format picture or video stream Looks the performance of the module is not bad, see some detection results on the link[3][4]. Face detection can be used to implement some interesting applications (camera, face unlock, baby monitor, ...). v2-v1: - extend face detection API to object detection API - extend face detection generic module to object detection module - introduce subdevice and media entity to object detection module - some minor fixes TODO: - implement OD setting interfaces with v4l2 controls or ext controls arch/arm/mach-omap2/devices.c | 33 + arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 81 +++ drivers/media/video/Kconfig|6 + drivers/media/video/Makefile |2 + drivers/media/video/odif/Kconfig | 13 + drivers/media/video/odif/Makefile |2 + drivers/media/video/odif/fdif_omap4.c | 685 + drivers/media/video/odif/odif.c| 890 drivers/media/video/odif/odif.h| 157 + drivers/media/video/v4l2-ioctl.c | 72 +++- drivers/media/video/videobuf2-dma-contig.c |1 + drivers/media/video/videobuf2-memops.c |1 - drivers/media/video/videobuf2-page.c | 117 include/linux/videodev2.h | 124 include/media/v4l2-ioctl.h |6 + include/media/videobuf2-page.h | 20 + 16 files changed, 2207 insertions(+), 3 deletions(-) thanks, -- Ming Lei [1], Ch9 of OMAP4 Technical Reference Manual [2], http://kernel.ubuntu.com/git?p=ming/fdif.git;a=shortlog;h=refs/heads/v4l2-fdif [3], http://kernel.ubuntu.com/~ming/dev/fdif/output [4], All pictures are taken from http://www.google.com/imghp and converted to pnm from jpeg format, only for test purpose. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 6/8] media: v4l2: introduce two IOCTLs for object detection
This patch introduces two new IOCTLs and related data structure which will be used by the coming video device with object detect capability. The two IOCTLs and related data structure will be used by user space application to retrieve the results of object detection. The utility fdif[1] is useing the two IOCTLs to find objects(faces) deteced in raw images or video streams. [1],http://kernel.ubuntu.com/git?p=ming/fdif.git;a=shortlog;h=refs/heads/v4l2-fdif Signed-off-by: Ming Lei ming@canonical.com --- v2: - extend face detection API to object detection API - introduce capability of V4L2_CAP_OBJ_DETECTION for object detection - 32/64 safe array parameter --- drivers/media/video/v4l2-ioctl.c | 41 - include/linux/videodev2.h| 124 ++ include/media/v4l2-ioctl.h |6 ++ 3 files changed, 170 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index ded8b72..575d445 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -2140,6 +2140,30 @@ static long __video_do_ioctl(struct file *file, dbgarg(cmd, index=%d, b-index); break; } + case VIDIOC_G_OD_RESULT: + { + struct v4l2_od_result *or = arg; + + if (!ops-vidioc_g_od_result) + break; + + ret = ops-vidioc_g_od_result(file, fh, or); + + dbgarg(cmd, index=%d, or-frm_seq); + break; + } + case VIDIOC_G_OD_COUNT: + { + struct v4l2_od_count *oc = arg; + + if (!ops-vidioc_g_od_count) + break; + + ret = ops-vidioc_g_od_count(file, fh, oc); + + dbgarg(cmd, index=%d, oc-frm_seq); + break; + } default: if (!ops-vidioc_default) break; @@ -2241,7 +2265,22 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, static int is_64_32_array_args(unsigned int cmd, void *parg, int *extra_len) { - return 0; + int ret = 0; + + switch (cmd) { + case VIDIOC_G_OD_RESULT: { + struct v4l2_od_result *or = parg; + + *extra_len = or-obj_cnt * + sizeof(struct v4l2_od_object); + ret = 1; + break; + } + default: + break; + } + + return ret; } long diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 4b752d5..c08ceaf 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -270,6 +270,9 @@ struct v4l2_capability { #define V4L2_CAP_RADIO 0x0004 /* is a radio device */ #define V4L2_CAP_MODULATOR 0x0008 /* has a modulator */ +/* The device has capability of object detection */ +#define V4L2_CAP_OBJ_DETECTION 0x0010 + #define V4L2_CAP_READWRITE 0x0100 /* read/write systemcalls */ #define V4L2_CAP_ASYNCIO0x0200 /* async I/O */ #define V4L2_CAP_STREAMING 0x0400 /* streaming I/O ioctls */ @@ -2160,6 +2163,125 @@ struct v4l2_create_buffers { __u32 reserved[8]; }; +/** + * struct v4l2_od_obj_desc + * @centerx: return, position in x direction of detected object + * @centery: return, position in y direction of detected object + * @sizex: return, size in x direction of detected object + * @sizey: return, size in y direction of detected object + * @angle: return, angle of detected object + * 0 deg ~ 359 deg, vertical is 0 deg, clockwise + * @reserved: future extensions + */ +struct v4l2_od_obj_desc { + __u16 centerx; + __u16 centery; + __u16 sizex; + __u16 sizey; + __u16 angle; + __u16 reserved[5]; +}; + +/** + * struct v4l2_od_face_desc + * @id:return, used to be associated with detected eyes, mouth, + * and other objects inside this face, and each face in one + * frame has a unique id, start from 1 + * @smile_level:return, smile level of the face + * @f: return, face description + */ +struct v4l2_od_face_desc { + __u16 id; + __u8smile_level; + __u8reserved[15]; + + struct v4l2_od_obj_desc f; +}; + +/** + * struct v4l2_od_eye_desc + * @face_id: return, used to associate with which face, 0 means + * no face associated with the eye + * @blink_level:return, blink level of the eye + * @e: return, eye description + */ +struct v4l2_od_eye_desc { + __u16 face_id; + __u8blink_level; + __u8reserved[15]; + + struct v4l2_od_obj_desc e; +}; + +/** + * struct v4l2_od_mouth_desc + * @face_id: return, used to associate
[RFC PATCH v2 1/8] omap4: introduce fdif(face detect module) hwmod
Signed-off-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 81 1 files changed, 81 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6cf21ee..30db754 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -53,6 +53,7 @@ static struct omap_hwmod omap44xx_dmm_hwmod; static struct omap_hwmod omap44xx_dsp_hwmod; static struct omap_hwmod omap44xx_dss_hwmod; static struct omap_hwmod omap44xx_emif_fw_hwmod; +static struct omap_hwmod omap44xx_fdif_hwmod; static struct omap_hwmod omap44xx_hsi_hwmod; static struct omap_hwmod omap44xx_ipu_hwmod; static struct omap_hwmod omap44xx_iss_hwmod; @@ -354,6 +355,14 @@ static struct omap_hwmod_ocp_if omap44xx_dma_system__l3_main_2 = { .user = OCP_USER_MPU | OCP_USER_SDMA, }; +/* fdif - l3_main_2 */ +static struct omap_hwmod_ocp_if omap44xx_fdif__l3_main_2 = { + .master = omap44xx_fdif_hwmod, + .slave = omap44xx_l3_main_2_hwmod, + .clk= l3_div_ck, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + /* hsi - l3_main_2 */ static struct omap_hwmod_ocp_if omap44xx_hsi__l3_main_2 = { .master = omap44xx_hsi_hwmod, @@ -5444,6 +5453,75 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = { .slaves_cnt = ARRAY_SIZE(omap44xx_wd_timer3_slaves), }; +/* 'fdif' class */ +static struct omap_hwmod_class_sysconfig omap44xx_fdif_sysc = { + .rev_offs = 0x, + .sysc_offs = 0x0010, + .sysc_flags = (SYSC_HAS_MIDLEMODE | SYSC_HAS_RESET_STATUS | + SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | + MSTANDBY_FORCE | MSTANDBY_NO | + MSTANDBY_SMART), + .sysc_fields= omap_hwmod_sysc_type2, +}; + +static struct omap_hwmod_class omap44xx_fdif_hwmod_class = { + .name = fdif, + .sysc = omap44xx_fdif_sysc, +}; + +/*fdif*/ +static struct omap_hwmod_addr_space omap44xx_fdif_addrs[] = { + { + .pa_start = 0x4a10a000, + .pa_end = 0x4a10afff, + .flags = ADDR_TYPE_RT + }, + { } +}; + +/* l4_cfg - fdif */ +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__fdif = { + .master = omap44xx_l4_cfg_hwmod, + .slave = omap44xx_fdif_hwmod, + .clk= l4_div_ck, + .addr = omap44xx_fdif_addrs, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +/* fdif slave ports */ +static struct omap_hwmod_ocp_if *omap44xx_fdif_slaves[] = { + omap44xx_l4_cfg__fdif, +}; +static struct omap_hwmod_irq_info omap44xx_fdif_irqs[] = { + { .irq = 69 + OMAP44XX_IRQ_GIC_START }, + { .irq = -1 } +}; + +/* fdif master ports */ +static struct omap_hwmod_ocp_if *omap44xx_fdif_masters[] = { + omap44xx_fdif__l3_main_2, +}; + +static struct omap_hwmod omap44xx_fdif_hwmod = { + .name = fdif, + .class = omap44xx_fdif_hwmod_class, + .clkdm_name = iss_clkdm, + .mpu_irqs = omap44xx_fdif_irqs, + .main_clk = fdif_fck, + .prcm = { + .omap4 = { + .clkctrl_offs = OMAP4_CM_CAM_FDIF_CLKCTRL_OFFSET, + .context_offs = OMAP4_RM_CAM_FDIF_CONTEXT_OFFSET, + .modulemode = MODULEMODE_SWCTRL, + }, + }, + .slaves = omap44xx_fdif_slaves, + .slaves_cnt = ARRAY_SIZE(omap44xx_fdif_slaves), + .masters= omap44xx_fdif_masters, + .masters_cnt= ARRAY_SIZE(omap44xx_fdif_masters), +}; + static __initdata struct omap_hwmod *omap44xx_hwmods[] = { /* dmm class */ @@ -5593,6 +5671,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = { omap44xx_wd_timer2_hwmod, omap44xx_wd_timer3_hwmod, + /* fdif class */ + omap44xx_fdif_hwmod, + NULL, }; -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 2/8] omap4: build fdif omap device from hwmod
Signed-off-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/devices.c | 33 + 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index 1166bdc..bd7f9b3 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -728,6 +728,38 @@ void __init omap242x_init_mmc(struct omap_mmc_platform_data **mmc_data) #endif +static __init struct platform_device *omap4_init_fdif(void) +{ + struct platform_device *pd; + struct omap_hwmod *oh; + const char *dev_name = omap-fdif; + + oh = omap_hwmod_lookup(fdif); + if (!oh) { + pr_err(Could not look up fdif hwmod\n); + return NULL; + } + + pd = omap_device_build(dev_name, -1, oh, NULL, 0, NULL, 0, 0); + WARN(IS_ERR(pd), Can't build omap_device for %s.\n, + dev_name); + return pd; +} + +static void __init omap_init_fdif(void) +{ + struct platform_device *pd; + + if (!cpu_is_omap44xx()) + return; + + pd = omap4_init_fdif(); + if (!pd) + return; + + pm_runtime_enable(pd-dev); +} + /*-*/ #if defined(CONFIG_HDQ_MASTER_OMAP) || defined(CONFIG_HDQ_MASTER_OMAP_MODULE) @@ -808,6 +840,7 @@ static int __init omap2_init_devices(void) omap_init_sham(); omap_init_aes(); omap_init_vout(); + omap_init_fdif(); return 0; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 3/8] media: videobuf2: move out of setting pgprot_noncached from vb2_mmap_pfn_range
So that we can reuse vb2_mmap_pfn_range for the coming videobuf2_page memops. Signed-off-by: Ming Lei ming@canonical.com --- drivers/media/video/videobuf2-dma-contig.c |1 + drivers/media/video/videobuf2-memops.c |1 - 2 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index f17ad98..0ea8866 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -106,6 +106,7 @@ static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma) return -EINVAL; } + vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot); return vb2_mmap_pfn_range(vma, buf-dma_addr, buf-size, vb2_common_vm_ops, buf-handler); } diff --git a/drivers/media/video/videobuf2-memops.c b/drivers/media/video/videobuf2-memops.c index 71a7a78..77e0def 100644 --- a/drivers/media/video/videobuf2-memops.c +++ b/drivers/media/video/videobuf2-memops.c @@ -162,7 +162,6 @@ int vb2_mmap_pfn_range(struct vm_area_struct *vma, unsigned long paddr, size = min_t(unsigned long, vma-vm_end - vma-vm_start, size); - vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot); ret = remap_pfn_range(vma, vma-vm_start, paddr PAGE_SHIFT, size, vma-vm_page_prot); if (ret) { -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 4/8] media: videobuf2: introduce VIDEOBUF2_PAGE memops
DMA contig memory resource is very limited and precious, also accessing to it from CPU is very slow on some platform. For some cases(such as the comming face detection driver), DMA Streaming buffer is enough, so introduce VIDEOBUF2_PAGE to allocate continuous physical memory but letting video device driver to handle DMA buffer mapping and unmapping things. Signed-off-by: Ming Lei ming@canonical.com --- drivers/media/video/Kconfig |4 + drivers/media/video/Makefile |1 + drivers/media/video/videobuf2-page.c | 117 ++ include/media/videobuf2-page.h | 20 ++ 4 files changed, 142 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/videobuf2-page.c create mode 100644 include/media/videobuf2-page.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 4e8a0c4..5684a00 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -60,6 +60,10 @@ config VIDEOBUF2_VMALLOC select VIDEOBUF2_MEMOPS tristate +config VIDEOBUF2_PAGE + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate config VIDEOBUF2_DMA_SG #depends on HAS_DMA diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index ddeaa6c..bc797f2 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -125,6 +125,7 @@ obj-$(CONFIG_VIDEO_BTCX) += btcx-risc.o obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC)+= videobuf2-vmalloc.o +obj-$(CONFIG_VIDEOBUF2_PAGE) += videobuf2-page.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o diff --git a/drivers/media/video/videobuf2-page.c b/drivers/media/video/videobuf2-page.c new file mode 100644 index 000..6a24a34 --- /dev/null +++ b/drivers/media/video/videobuf2-page.c @@ -0,0 +1,117 @@ +/* + * videobuf2-page.c - page memory allocator for videobuf2 + * + * Copyright (C) 2011 Canonical Ltd. + * + * Author: Ming Lei ming@canonical.com + * + * This file is based on videobuf2-vmalloc.c + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation. + */ + +#include linux/module.h +#include linux/mm.h +#include linux/slab.h + +#include media/videobuf2-core.h +#include media/videobuf2-memops.h + +struct vb2_page_buf { + void*vaddr; + unsigned long size; + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_page_put(void *buf_priv); + +static void *vb2_page_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_page_buf *buf; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return NULL; + + buf-size = size; + buf-vaddr = (void *)__get_free_pages(GFP_KERNEL, + get_order(buf-size)); + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_page_put; + buf-handler.arg = buf; + + if (!buf-vaddr) { + printk(KERN_ERR page of size %ld failed\n, buf-size); + kfree(buf); + return NULL; + } + + atomic_inc(buf-refcount); + printk(KERN_DEBUG Allocated page buffer of size %ld at vaddr=%p\n, + buf-size, buf-vaddr); + + return buf; +} + +static void vb2_page_put(void *buf_priv) +{ + struct vb2_page_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + printk(KERN_DEBUG %s: Freeing page mem at vaddr=%p\n, + __func__, buf-vaddr); + free_pages((unsigned long)buf-vaddr, get_order(buf-size)); + kfree(buf); + } +} + +static void *vb2_page_vaddr(void *buf_priv) +{ + struct vb2_page_buf *buf = buf_priv; + + BUG_ON(!buf); + + if (!buf-vaddr) { + printk(KERN_ERR Address of an unallocated plane requested\n); + return NULL; + } + + return buf-vaddr; +} + +static unsigned int vb2_page_num_users(void *buf_priv) +{ + struct vb2_page_buf *buf = buf_priv; + return atomic_read(buf-refcount); +} + +static int vb2_page_mmap(void *buf_priv, struct vm_area_struct *vma) +{ + struct vb2_page_buf *buf = buf_priv; + + if (!buf) { + printk(KERN_ERR No memory to map\n); + return -EINVAL; + } + + vma-vm_page_prot = vm_get_page_prot(vma-vm_flags); + return vb2_mmap_pfn_range(vma, virt_to_phys(buf-vaddr), + buf-size, vb2_common_vm_ops, + buf-handler); +} + +const struct vb2_mem_ops vb2_page_memops = { + .alloc
[RFC PATCH v2 5/8] media: v4l2-ioctl: support 64/32 compatible array parameter
This patch supports to handle 64/32 compatible array parameter, such as below: struct v4l2_fd_result { __u32 buf_index; __u32 face_cnt; __u32 reserved[6]; struct v4l2_fd_detection fd[]; }; With this patch, the pointer to user space array needn't be passed to kernel any more. Cc: Arnd Bergmann a...@arndb.de Signed-off-by: Ming Lei ming@canonical.com --- drivers/media/video/v4l2-ioctl.c | 33 +++-- 1 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index e1da8fc..ded8b72 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -2239,6 +2239,11 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, return ret; } +static int is_64_32_array_args(unsigned int cmd, void *parg, int *extra_len) +{ + return 0; +} + long video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, v4l2_kioctl func) @@ -2251,6 +2256,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, size_t array_size = 0; void __user *user_ptr = NULL; void**kernel_ptr = NULL; + int extra = 0; /* Copy arguments into temp kernel buffer */ if (_IOC_DIR(cmd) != _IOC_NONE) { @@ -2280,9 +2286,32 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, } } - err = check_array_args(cmd, parg, array_size, user_ptr, kernel_ptr); + if (is_64_32_array_args(cmd, parg, extra)) { + int size; + void *old_mbuf; + + err = 0; + if (!extra) + goto handle_array_args; + old_mbuf = mbuf; + size = extra + _IOC_SIZE(cmd); + mbuf = kmalloc(size, GFP_KERNEL); + if (NULL == mbuf) { + mbuf = old_mbuf; + err = -ENOMEM; + goto out; + } + memcpy(mbuf, parg, _IOC_SIZE(cmd)); + parg = mbuf; + kfree(old_mbuf); + } else { + err = check_array_args(cmd, parg, array_size, + user_ptr, kernel_ptr); + } + if (err 0) goto out; +handle_array_args: has_array_args = err; if (has_array_args) { @@ -2321,7 +2350,7 @@ out_array_args: switch (_IOC_DIR(cmd)) { case _IOC_READ: case (_IOC_WRITE | _IOC_READ): - if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) + if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd) + extra)) err = -EFAULT; break; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 7/8] media: video: introduce object detection driver module
This patch introduces object detection generic driver. The driver is responsible for all v4l2 stuff, buffer management and other general things, and doesn't touch object detection hardware directly. Several interfaces are exported to low level drivers (such as the coming omap4 FD driver) which will communicate with object detection hw module. So the driver will make driving object detection hw modules more easy. TODO: - implement object detection setting interfaces with v4l2 controls or ext controls Signed-off-by: Ming Lei ming@canonical.com --- v2: - extend face detection driver to object detection driver - introduce subdevice and media entity - provide support to detect object from media HW --- drivers/media/video/Kconfig |2 + drivers/media/video/Makefile |1 + drivers/media/video/odif/Kconfig |7 + drivers/media/video/odif/Makefile |1 + drivers/media/video/odif/odif.c | 890 + drivers/media/video/odif/odif.h | 157 +++ 6 files changed, 1058 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/odif/Kconfig create mode 100644 drivers/media/video/odif/Makefile create mode 100644 drivers/media/video/odif/odif.c create mode 100644 drivers/media/video/odif/odif.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 5684a00..8740ee9 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -1166,3 +1166,5 @@ config VIDEO_SAMSUNG_S5P_MFC MFC 5.1 driver for V4L2. endif # V4L_MEM2MEM_DRIVERS + +source drivers/media/video/odif/Kconfig diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index bc797f2..259c8d8 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -197,6 +197,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o obj-y += davinci/ obj-$(CONFIG_ARCH_OMAP)+= omap/ +obj-$(CONFIG_ODIF) += odif/ ccflags-y += -Idrivers/media/dvb/dvb-core ccflags-y += -Idrivers/media/dvb/frontends diff --git a/drivers/media/video/odif/Kconfig b/drivers/media/video/odif/Kconfig new file mode 100644 index 000..5090bd6 --- /dev/null +++ b/drivers/media/video/odif/Kconfig @@ -0,0 +1,7 @@ +config ODIF + depends on VIDEO_DEV VIDEO_V4L2 + select VIDEOBUF2_PAGE + tristate Object Detection module + help + The ODIF is a object detection module, which can be integrated into + some SoCs to detect objects in images or video. diff --git a/drivers/media/video/odif/Makefile b/drivers/media/video/odif/Makefile new file mode 100644 index 000..a55ff66 --- /dev/null +++ b/drivers/media/video/odif/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_ODIF) += odif.o diff --git a/drivers/media/video/odif/odif.c b/drivers/media/video/odif/odif.c new file mode 100644 index 000..381ab9d --- /dev/null +++ b/drivers/media/video/odif/odif.c @@ -0,0 +1,890 @@ +/* + * odif.c -- object detection module driver + * + * Copyright (C) 2011 Ming Lei (ming@canonical.com) + * + * This file is based on drivers/media/video/vivi.c. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +/*/ + +#include linux/module.h +#include linux/fs.h +#include linux/mm.h +#include linux/signal.h +#include linux/wait.h +#include linux/poll.h +#include linux/mman.h +#include linux/pm_runtime.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/interrupt.h +#include asm/uaccess.h +#include asm/byteorder.h +#include asm/io.h +#include odif.h + +#defineinput_from_user(dev) \ + (dev-input == OD_INPUT_FROM_USER_SPACE) + +#defineDEFAULT_PENDING_RESULT_CNT 8 + +static unsigned debug = 0; +module_param(debug, uint, 0644); +MODULE_PARM_DESC(debug, activates debug info); + +static unsigned result_cnt_threshold = DEFAULT_PENDING_RESULT_CNT; +module_param(result_cnt_threshold, uint, 0644); +MODULE_PARM_DESC(result_cnt, max pending result count); + +static LIST_HEAD(odif_devlist); +static unsigned video_nr = -1; + +int odif_open(struct file *file) +{ + struct odif_dev *dev = video_drvdata(file); + + kref_get(dev
[RFC PATCH v2 8/8] media: video: introduce omap4 face detection module driver
The patch introduces one face detection device driver for driving face detection hardware on omap4[1]. Most things of the driver are dealing with omap4 face detection hardware. This driver is platform independent, so in theory it can be used to drive same IP module on other platforms. [1], Ch9 of OMAP4 Technical Reference Manual Signed-off-by: Ming Lei ming@canonical.com --- v2: - based on odif module - use new object detection API --- drivers/media/video/odif/Kconfig |6 + drivers/media/video/odif/Makefile |1 + drivers/media/video/odif/fdif_omap4.c | 685 + 3 files changed, 692 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/odif/fdif_omap4.c diff --git a/drivers/media/video/odif/Kconfig b/drivers/media/video/odif/Kconfig index 5090bd6..2a8c545 100644 --- a/drivers/media/video/odif/Kconfig +++ b/drivers/media/video/odif/Kconfig @@ -5,3 +5,9 @@ config ODIF help The ODIF is a object detection module, which can be integrated into some SoCs to detect objects in images or video. + +config ODIF_OMAP4 + depends on ODIF + tristate OMAP4 Face Detection module + help + OMAP4 face detection support diff --git a/drivers/media/video/odif/Makefile b/drivers/media/video/odif/Makefile index a55ff66..0eb844f 100644 --- a/drivers/media/video/odif/Makefile +++ b/drivers/media/video/odif/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_ODIF) += odif.o +obj-$(CONFIG_ODIF_OMAP4) += fdif_omap4.o diff --git a/drivers/media/video/odif/fdif_omap4.c b/drivers/media/video/odif/fdif_omap4.c new file mode 100644 index 000..d7953d8 --- /dev/null +++ b/drivers/media/video/odif/fdif_omap4.c @@ -0,0 +1,685 @@ +/* + * fdif_omap4.c -- face detection module driver + * + * Copyright (C) 2011 Ming Lei (ming@canonical.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +/*/ +#include linux/init.h +#include linux/fs.h +#include linux/mm.h +#include linux/slab.h +#include linux/signal.h +#include linux/wait.h +#include linux/poll.h +#include linux/module.h +#include linux/pm_runtime.h +#include linux/delay.h +#include linux/user_namespace.h +#include linux/platform_device.h +#include linux/interrupt.h +#include linux/dma-mapping.h +#include odif.h +#include asm/uaccess.h +#include asm/byteorder.h +#include asm/io.h + +#undef DEBUG + +#define PICT_SIZE_X 320 +#define PICT_SIZE_Y 240 + +#defineWORK_MEM_SIZE (52*1024) + +/* 9.5 FDIF Register Manua of TI OMAP4 TRM */ +#define FDIF_REVISION 0x0 +#define FDIF_HWINFO0x4 +#define FDIF_SYSCONFIG 0x10 +#define SOFTRESET (1 0) + +#define FDIF_IRQSTATUS_RAW_j (0x24 + 2*0x10) +#define FDIF_IRQSTATUS_j (0x28 + 2*0x10) +#define FDIF_IRQENABLE_SET_j (0x2c + 2*0x10) +#define FDIF_IRQENABLE_CLR_j (0x30 + 2*0x10) +#define FINISH_IRQ (1 8) +#define ERR_IRQ(1 0) + +#define FDIF_PICADDR 0x60 +#define FDIF_CTRL 0x64 +#define CTRL_MAX_TAGS 0x0A + +#define FDIF_WKADDR0x68 +#define FD_CTRL0x80 +#define CTRL_FINISH(1 2) +#define CTRL_RUN (1 1) +#define CTRL_SRST (1 0) + + +#define FD_DNUM0x84 +#define FD_DCOND 0x88 +#define FD_STARTX 0x8c +#define FD_STARTY 0x90 +#define FD_SIZEX 0x94 +#define FD_SIZEY 0x98 +#define FD_LHIT0x9c +#define FD_CENTERX_i 0x160 +#define FD_CENTERY_i 0x164 +#define FD_CONFSIZE_i 0x168 +#define FD_ANGLE_i 0x16c + +static inline void fd_writel(void __iomem *base, u32 reg, u32 val) +{ + __raw_writel(val, base + reg); +} + +static inline u32 fd_readl(void __iomem *base, u32 reg) +{ + return __raw_readl(base + reg); +} + +struct fdif_qvga { + struct odif_dev *dev; + + /*should be removed*/ + struct platform_device *pdev; + int irq; + void __iomem*base; + + void
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki snj...@gmail.com For OMAP4 FD, it is not needed to include FD into MC framework since a intermediate buffer is always required. If your HW doesn't belong to this case, what is the output of your HW FD in the link? Also sounds FD results may not be needed at all for use space application in the case. The result data is similar to OMAP4 one, plus a few other attributes. User buffers may be filled by other than FD device driver. OK. Could you provide some practical use cases about these? As above, and any device with a camera that controls something and makes decision according to presence of human face in his view. Sounds a reasonable case, :-) If FD result is associated with a frame, how can user space get the frame seq if no v4l2 buffer is involved? Without a frame sequence, it is a bit difficult to retrieve FD results from user space. If you pass image data in memory buffers from user space, yes, it could be impossible. It is easy to get the frame sequence from v4l2_buffer for the case too, :-) Not really, still v4l2_buffer may be used by other (sub)driver within same video processing pipeline. OK. A related question: how can we make one application to support the two kinds of devices(input from user space data as OMAP4, input from SoC bus as Samsung) at the same time? Maybe some capability info is to be exported to user space? or other suggestions? And will your Samsung FD HW support to detect faces from memory? or just only detect from SoC bus? It will be included in the FD result... or in a dedicated v4l2 event data structure. More important, at the end of the day, we'll be getting buffers with image data at some stage of a video pipeline, which would contain same frame identifier (I think we can ignore v4l2_buffer.field for FD purpose). OK, I will associate FD result with frame identifier, and not invent a dedicated v4l2 event for query frame seq now until a specific requirement for it is proposed. I will convert/integrate recent discussions into patches of v2 for further review, and sub device support will be provided. But before starting to do it, I am still not clear how to integrate FD into MC framework. I understand FD sub device is only a media entity, so how can FD sub device find the media device(struct media_device)? or just needn't to care about it now? thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 1/7] omap4: introduce fdif(face detect module) hwmod
Hi guys, Gentle ping on this patch, :-) thanks, -- Ming Lei On Fri, Dec 2, 2011 at 5:12 PM, Ming Lei ming@canonical.com wrote: Signed-off-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 81 1 files changed, 81 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6cf21ee..30db754 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -53,6 +53,7 @@ static struct omap_hwmod omap44xx_dmm_hwmod; static struct omap_hwmod omap44xx_dsp_hwmod; static struct omap_hwmod omap44xx_dss_hwmod; static struct omap_hwmod omap44xx_emif_fw_hwmod; +static struct omap_hwmod omap44xx_fdif_hwmod; static struct omap_hwmod omap44xx_hsi_hwmod; static struct omap_hwmod omap44xx_ipu_hwmod; static struct omap_hwmod omap44xx_iss_hwmod; @@ -354,6 +355,14 @@ static struct omap_hwmod_ocp_if omap44xx_dma_system__l3_main_2 = { .user = OCP_USER_MPU | OCP_USER_SDMA, }; +/* fdif - l3_main_2 */ +static struct omap_hwmod_ocp_if omap44xx_fdif__l3_main_2 = { + .master = omap44xx_fdif_hwmod, + .slave = omap44xx_l3_main_2_hwmod, + .clk = l3_div_ck, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + /* hsi - l3_main_2 */ static struct omap_hwmod_ocp_if omap44xx_hsi__l3_main_2 = { .master = omap44xx_hsi_hwmod, @@ -5444,6 +5453,75 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = { .slaves_cnt = ARRAY_SIZE(omap44xx_wd_timer3_slaves), }; +/* 'fdif' class */ +static struct omap_hwmod_class_sysconfig omap44xx_fdif_sysc = { + .rev_offs = 0x, + .sysc_offs = 0x0010, + .sysc_flags = (SYSC_HAS_MIDLEMODE | SYSC_HAS_RESET_STATUS | + SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | + MSTANDBY_FORCE | MSTANDBY_NO | + MSTANDBY_SMART), + .sysc_fields = omap_hwmod_sysc_type2, +}; + +static struct omap_hwmod_class omap44xx_fdif_hwmod_class = { + .name = fdif, + .sysc = omap44xx_fdif_sysc, +}; + +/*fdif*/ +static struct omap_hwmod_addr_space omap44xx_fdif_addrs[] = { + { + .pa_start = 0x4a10a000, + .pa_end = 0x4a10afff, + .flags = ADDR_TYPE_RT + }, + { } +}; + +/* l4_cfg - fdif */ +static struct omap_hwmod_ocp_if omap44xx_l4_cfg__fdif = { + .master = omap44xx_l4_cfg_hwmod, + .slave = omap44xx_fdif_hwmod, + .clk = l4_div_ck, + .addr = omap44xx_fdif_addrs, + .user = OCP_USER_MPU | OCP_USER_SDMA, +}; + +/* fdif slave ports */ +static struct omap_hwmod_ocp_if *omap44xx_fdif_slaves[] = { + omap44xx_l4_cfg__fdif, +}; +static struct omap_hwmod_irq_info omap44xx_fdif_irqs[] = { + { .irq = 69 + OMAP44XX_IRQ_GIC_START }, + { .irq = -1 } +}; + +/* fdif master ports */ +static struct omap_hwmod_ocp_if *omap44xx_fdif_masters[] = { + omap44xx_fdif__l3_main_2, +}; + +static struct omap_hwmod omap44xx_fdif_hwmod = { + .name = fdif, + .class = omap44xx_fdif_hwmod_class, + .clkdm_name = iss_clkdm, + .mpu_irqs = omap44xx_fdif_irqs, + .main_clk = fdif_fck, + .prcm = { + .omap4 = { + .clkctrl_offs = OMAP4_CM_CAM_FDIF_CLKCTRL_OFFSET, + .context_offs = OMAP4_RM_CAM_FDIF_CONTEXT_OFFSET, + .modulemode = MODULEMODE_SWCTRL, + }, + }, + .slaves = omap44xx_fdif_slaves, + .slaves_cnt = ARRAY_SIZE(omap44xx_fdif_slaves), + .masters = omap44xx_fdif_masters, + .masters_cnt = ARRAY_SIZE(omap44xx_fdif_masters), +}; + static __initdata struct omap_hwmod *omap44xx_hwmods[] = { /* dmm class */ @@ -5593,6 +5671,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = { omap44xx_wd_timer2_hwmod, omap44xx_wd_timer3_hwmod, + /* fdif class */ + omap44xx_fdif_hwmod, + NULL, }; thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, On Mon, Dec 12, 2011 at 8:08 PM, HeungJun, Kim riverful@samsung.com wrote: Hi Ming, It's maybe late, but I want to suggest one thing about FD API. This OMAP FD block looks detection ability of just face. But, It's possible to occur another device which can detect specific object or patterns. Moreover, this API can expand object recognition area. So, I think it's good to change the API name like v4l2_recog. IMO, object detection is better, at least now OMAP4 and samsung has face detection IP module, and face recognition is often done on results of face detection and more complicated interfaces will be involved. Actually, I'm preparing similar control class for mainline with m5mols camera sensor driver. The m5mols camera sensor has the function about face detection. But, I has experienced about Robot Recognition, and I remember the image processing chip which can detect spefic pattern. So, I hesitated naming the API(control or ioctl whatever) with face. It can be possible to provide just object or pattern, not face. Even user library on windows, there is famous OpenCV. And this is also support not only face, but also object. Yes, object is better than face, and we can use enum flag to describe that the objects detected are which kind of objects. In fact, I plan to rename the face detection generic driver as object detection generic driver and let hardware driver to handle the object detection details. The function of OMAP FDIF looks like m5mols ISP's one. please understand I don't have experience about OMAP AP. But, I can tell you it's better to use the name object recognition, not the face detection, for any other device or driver. In a few days, I'll share the CIDs I have thought for m5mols driver. And, I hope to discuss about this with OMAP FDIF. You have been doing it already, :-) thanks, -- Ming Lei Thank you. Regards, Heungjun Kim -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Ming Lei Sent: Monday, December 12, 2011 6:50 PM To: Sylwester Nawrocki Cc: linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; linux-me...@vger.kernel.org Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module Hi, On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki snj...@gmail.com For OMAP4 FD, it is not needed to include FD into MC framework since a intermediate buffer is always required. If your HW doesn't belong to this case, what is the output of your HW FD in the link? Also sounds FD results may not be needed at all for use space application in the case. The result data is similar to OMAP4 one, plus a few other attributes. User buffers may be filled by other than FD device driver. OK. Could you provide some practical use cases about these? As above, and any device with a camera that controls something and makes decision according to presence of human face in his view. Sounds a reasonable case, :-) If FD result is associated with a frame, how can user space get the frame seq if no v4l2 buffer is involved? Without a frame sequence, it is a bit difficult to retrieve FD results from user space. If you pass image data in memory buffers from user space, yes, it could be impossible. It is easy to get the frame sequence from v4l2_buffer for the case too, :-) Not really, still v4l2_buffer may be used by other (sub)driver within same video processing pipeline. OK. A related question: how can we make one application to support the two kinds of devices(input from user space data as OMAP4, input from SoC bus as Samsung) at the same time? Maybe some capability info is to be exported to user space? or other suggestions? And will your Samsung FD HW support to detect faces from memory? or just only detect from SoC bus? It will be included in the FD result... or in a dedicated v4l2 event data structure. More important, at the end of the day, we'll be getting buffers with image data at some stage of a video pipeline, which would contain same frame identifier (I think we can ignore v4l2_buffer.field for FD purpose). OK, I will associate FD result with frame identifier, and not invent a dedicated v4l2 event for query frame seq now until a specific requirement for it is proposed. I will convert/integrate recent discussions into patches of v2 for further review, and sub device support will be provided. But before starting to do it, I am still not clear how to integrate FD into MC framework. I understand FD sub device is only a media entity, so how can FD sub device find the media device(struct media_device)? or just needn't to care about it now? thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
On Tue, Dec 13, 2011 at 1:59 PM, HeungJun, Kim riverful@samsung.com wrote: Hi Ming and Sylwester, Thanks for the reply. -Original Message- From: Ming Lei [mailto:ming@canonical.com] Sent: Tuesday, December 13, 2011 1:02 PM To: HeungJun, Kim Cc: Sylwester Nawrocki; linux-omap@vger.kernel.org; linux-arm- ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux- me...@vger.kernel.org Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module Hi, On Mon, Dec 12, 2011 at 8:08 PM, HeungJun, Kim riverful@samsung.com wrote: Hi Ming, It's maybe late, but I want to suggest one thing about FD API. This OMAP FD block looks detection ability of just face. But, It's possible to occur another device which can detect specific object or patterns. Moreover, this API can expand object recognition area. So, I think it's good to change the API name like v4l2_recog. IMO, object detection is better, at least now OMAP4 and samsung has face detection IP module, and face recognition is often done on results of face detection and more complicated interfaces will be involved. Actually, the detection point is different, I guess. The OMAP has the detection block separately, named FDIF. But, Samsung Exynos doing detection process with externel sensor - m5mols, in our case. This sensor(m5mols) has ISP function, and these ISP can process detection. The expert of FIMC is Sylwester. Probably, he can tell the differences between both in more details. :) Actually, I'm preparing similar control class for mainline with m5mols camera sensor driver. The m5mols camera sensor has the function about face detection. But, I has experienced about Robot Recognition, and I remember the image processing chip which can detect spefic pattern. So, I hesitated naming the API(control or ioctl whatever) with face. It can be possible to provide just object or pattern, not face. Even user library on windows, there is famous OpenCV. And this is also support not only face, but also object. Yes, object is better than face, and we can use enum flag to describe that the objects detected are which kind of objects. In fact, I plan to rename the face detection generic driver as object detection generic driver and let hardware driver to handle the object detection details. The function of OMAP FDIF looks like m5mols ISP's one. please understand I don't have experience about OMAP AP. But, I can tell you it's better to use the name object recognition, not the face detection, for any other device or driver. In a few days, I'll share the CIDs I have thought for m5mols driver. And, I hope to discuss about this with OMAP FDIF. You have been doing it already, :-) Yeah, actually I did. :) But, until I see OMAP FDIF case, I did not recognize AP(like OMAP) can have detection capability. :) So, although I did not think about at that time, I also think we should re-consider this case for now. As I look around your patch quickly, the functions is very similar with ours. (even detection of left/right eye) So, the problem is there are two ways to proceed recognition. - Does it handle at the level of IOCTLs? or CIDs? The patch introduces two IOCTL to retrieve object detection result. I think it should be handled by IOCTL. The interface is a little complex, so it is not easy to handle it by CIDs, IMO. If the detection algorithm is proceeded at the level of HW block, it's right the platform driver provide APIs as IOCTLs(as you're FDIF patches). On the other hand, if it is proceeded at the sensor(subdevice) level, I think it's more right to control using CIDs. Certainly, some generic CIDs for object detection will be introduced later and will be handled in the object detection(the current fdif generic driver, patch 6/7) generic driver. We need the solution including those two cases. And the name - object detection? object recognition? I think object detection is better. For example, face detection is very different with face recognition, isn't it? thanks, -- Ming Lei So, do you have any good ideas? ps. There can be another not-matched HW block level issues. But, the problem which I can check is just above issue for now. Thanks, Heungjun Kim thanks, -- Ming Lei Thank you. Regards, Heungjun Kim -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Ming Lei Sent: Monday, December 12, 2011 6:50 PM To: Sylwester Nawrocki Cc: linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; linux-me...@vger.kernel.org Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module Hi, On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki snj...@gmail.com For OMAP4 FD, it is not needed to include FD into MC framework since a intermediate buffer
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
On Fri, Dec 9, 2011 at 7:25 AM, Sylwester Nawrocki snj...@gmail.com wrote: On 12/07/2011 02:40 PM, Ming Lei wrote: I understand the API you mentioned here should belong to kernel internal API, correct me if it is wrong. Yes, I meant the in kernel design, i.e. generic face detection kernel module and an OMAP4 FDIF driver. It makes lots of sense to separate common code in this way, maybe even when there would be only OMAP devices using it. Yes, that is the motivation of the generic FD module. I think we can focus on two use cases for the generic FD now: - one is to detect faces from user space image data - another one is to detect faces in image data generated from HW(SoC internal bus, resize hardware) For OMAP4 hardware, input data is always from physically continuous memory directly, so it is very easy to support the two cases. For the use case 2, if buffer copy is to be avoided, we can use the coming shared dma-buf[1] to pass the image buffer produced by other HW to FD hw directly. Some H/W uses direct data buses to exchange data between processing blocks, and there is no need for additional memory. We only need to manage the data links, for which MC has been designed. For OMAP4 FD, it is not needed to include FD into MC framework since a intermediate buffer is always required. If your HW doesn't belong to this case, what is the output of your HW FD in the link? Also sounds FD results may not be needed at all for use space application in the case. For other FD hardware, if it supports to detect faces in image data from physically continuous memory, I think the patch is OK to support it. If the FD hw doesn't support to detect faces from physically continuous memory, I have some questions: how does user space app to parse the FD result if application can't get the input image data? If user space can Do we need the map of detected objects on a input image in all cases ? For normal cases, I think we need, :-) If an application needs only coordinates of detected object on a video signal to for example, focus on it, trigger some action, or just count detected faces, etc. Perhaps there are more practical similar use cases. Could you provide some practical use cases about these? get image data, how does it connect the image data with FD result? and If hardware provides frame sequence numbers the FD result can be associated with a frame, whether it's passing through H/W interconnect or is located in memory. If FD result is associated with a frame, how can user space get the frame seq if no v4l2 buffer is involved? Without a frame sequence, it is a bit difficult to retrieve FD results from user space. what standard v4l2 ways(v4l2_buffer?) can the app use to describe the image data? We have USERPTR and MMAP memeory buffer for streaming IO, those use v4l2_buffer 1). read()/write() is also used 2), mostly for compressed formats. Except that there are works on shared buffers. If the input image data is from other HW(SoC bus, resizer HW, ...), is the v4l2_buffer needed for the FD HW driver or application? I'm sure now the Samsung devices won't fit in video output node based driver design. They read image data in different ways and also the FD result format is totally different. I think user space will need the FD result, so it is very important to define API to describe the FD result format to user space. And the input about your FD HW result format is certainly helpful to define the API. I'll post exact attributes generated by our FD detection H/W soon. Good news, :-) AFAICS OMAP4 FDIF processes only data stored in memory, thus it seems reasonable to use the videodev interface for passing data to the kernel from user space. But there might be face detection devices that accept data from other H/W modules, e.g. transferred through SoC internal data buses between image processing pipeline blocks. Thus any new interfaces need to be designed with such devices in mind. Also the face detection hardware block might now have an input DMA engine in it, the data could be fed from memory through some other subsystem (e.g. resize/colour converter). Then the driver for that subsystem would implement a video node. I think the direct input image or frame data to FD should be from memory no matter the actual data is from external H/W modules or input DMA because FD will take lot of time to detect faces in one image or frame and FD can't have so much memory to cache several images or frames data. Sorry, I cannot provide much details at the moment, but there exists hardware that reads data from internal SoC buses and even if it uses some sort of cache memory it doesn't necessarily have to be available for the user. Without some hardware background, it is not easy to give a generic FD module design. Yes, please give me some time so I can prepare the list of requirements. Still the FD result is associated with an image frame
Re: [RFC PATCH v1 5/7] media: v4l2: introduce two IOCTLs for face detection
Hi, On Fri, Dec 9, 2011 at 6:27 AM, Sylwester Nawrocki snj...@gmail.com wrote: On 12/08/2011 04:42 AM, Ming Lei wrote: +/** + * struct v4l2_obj_detection + * @buf_index: entry, index of v4l2_buffer for face detection I would prefer having the frame sequence number here. It will be more future proof IMHO. If for instance we decide to use such an ioctl on a v4l2 sub-device, without dequeuing buffers, there will be no problem with that. And still in your specific use case it's not big deal to look up the buffer index given it's sequence number in the application. OK, take your suggestion to use frame index, but I still have question about it, see my question in another thread. + * @centerx: return, position in x direction of detected object + * @centery: return, position in y direction of detected object + * @angle: return, angle of detected object + * 0 deg ~ 359 deg, vertical is 0 deg, clockwise + * @sizex: return, size in x direction of detected object + * @sizey: return, size in y direction of detected object + * @confidence: return, confidence level of detection result + * 0: the heighest level, 9: the lowest level Hmm, not a good idea to align a public interface to the capabilities of a single hardware implementation. I think that the current omap interface is general enough, so why can't we use it as public interface? I meant exactly the line implying the range. What if for some hardware it's 0..11 ? We can let driver to normalize it to user which doesn't care if the range is 0~11 or 10~21, a uniform range should always make user happy, shouldn't it? min/max confidence could be queried with relevant controls and here we could remove the line implying range. No, the confidence is used to describe the probability about the correctness of the current detection result. Anyway, no FD can make sure that it is 100% correct. Other HW can normalize its confidence level to 0~9 so that application can handle it easily, IMO. 1..100 might be better, to minimize rounding errors. Nevertheless IMO if we can export an exact range supported by FD device we should do it, and let upper layers do the normalization. And the bigger numbers should mean higher confidence, consistently for all devices. Looks 1..100 is better, and I will change it to 1..100. Do you think we could assume that the FD threshold range (FD_LHIT register in case of OMAP4) is always same as the result confidence level ? No, they are different. FD_LHIT is used to guild FD HW to detect more faces but more false positives __or__ less faces but less false positives. A control class is needed to be introduced for adjusting this value of FD HW, and I think a normalized range is better too. If so then the confidence level range could possibly be queried with the detection threshold control. We could name it V4L2_CID_FD_CONFIDENCE_THRESHOLD As I said above, there is no advantage to export the range to user, and a uniform range will make user happy. for example. I could take care of preparing the control class draft and the documentation for it. It is great to hear it, :-) + * @reserved: future extensions + */ +struct v4l2_obj_detection { How about changing name of this structure to v4l2_fd_primitive or v4l2_fd_shape ? I think v4l2_obj_detection is better because it can be reused to describe some other kind of object detection from video in the future. + __u16 centerx; + __u16 centery; + __u16 angle; + __u16 sizex; + __u16 sizey; How about using struct v4l2_rect in place of centerx/centery, sizex/sizey ? After all it describes a rectangle. We could also use struct v4l2_frmsize_discrete for size but there seems to be missing en equivalent for position, e.g. Maybe user space would like to plot a circle or ellipse over the detected objection, and I am sure that I have seen this kind of plot over detected face before. OK, in any way I suggest to replace all __u16 with __u32, to minimize performance issues and be consistent with the data type specifying pixel values elsewhere in V4L. OK, but may introduce more memory footprint for the fd result. It makes sense to make 'confidence' __u32 as well and add a flags attribute to indicate the shape. Sounds good. + __u16 confidence; + __u32 reserved[4]; And then __u32 reserved[10]; or __u32 reserved[2]; +}; + +#define V4L2_FD_HAS_LEFT_EYE 0x1 +#define V4L2_FD_HAS_RIGHT_EYE 0x2 +#define V4L2_FD_HAS_MOUTH 0x4 +#define V4L2_FD_HAS_FACE 0x8 Do you think we could change it to: #define V4L2_FD_FL_LEFT_EYE (1 0) #define V4L2_FD_FL_RIGHT_EYE (1 1) #define V4L2_FD_FL_MOUTH (1 2) #define V4L2_FD_FL_FACE (1 3) OK and add: #define V4L2_FD_FL_SMILE (1 4) #define
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, On Wed, Dec 7, 2011 at 6:01 AM, Sylwester Nawrocki snj...@gmail.com wrote: On 12/06/2011 03:07 PM, Ming Lei wrote: Hi, Thanks for your review. On Tue, Dec 6, 2011 at 5:55 AM, Sylwester Nawrocki snj...@gmail.com wrote: Hi Ming, (I've pruned the Cc list, leaving just the mailing lists) On 12/02/2011 04:02 PM, Ming Lei wrote: This patch introduces one driver for face detection purpose. The driver is responsible for all v4l2 stuff, buffer management and other general things, and doesn't touch face detection hardware directly. Several interfaces are exported to low level drivers (such as the coming omap4 FD driver)which will communicate with face detection hw module. So the driver will make driving face detection hw modules more easy. I would hold on for a moment on implementing generic face detection module which is based on the V4L2 video device interface. We need to first define an API that would be also usable at sub-device interface level (http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html). If we can define a good/stable enough APIs between kernel and user space, I think the patches can be merged first. For internal kernel APIs, we should allow it to evolve as new hardware comes or new features are to be introduced. I also don't see a problem in discussing it a bit more;) OK, fair enough, let's discuss it, :-) I understand the API you mentioned here should belong to kernel internal API, correct me if it is wrong. Yes, I meant the in kernel design, i.e. generic face detection kernel module and an OMAP4 FDIF driver. It makes lots of sense to separate common code in this way, maybe even when there would be only OMAP devices using it. Yes, that is the motivation of the generic FD module. I think we can focus on two use cases for the generic FD now: - one is to detect faces from user space image data - another one is to detect faces in image data generated from HW(SoC internal bus, resize hardware) For OMAP4 hardware, input data is always from physically continuous memory directly, so it is very easy to support the two cases. For the use case 2, if buffer copy is to be avoided, we can use the coming shared dma-buf[1] to pass the image buffer produced by other HW to FD hw directly. For other FD hardware, if it supports to detect faces in image data from physically continuous memory, I think the patch is OK to support it. If the FD hw doesn't support to detect faces from physically continuous memory, I have some questions: how does user space app to parse the FD result if application can't get the input image data? If user space can get image data, how does it connect the image data with FD result? and what standard v4l2 ways(v4l2_buffer?) can the app use to describe the image data? I'm sure now the Samsung devices won't fit in video output node based driver design. They read image data in different ways and also the FD result format is totally different. I think user space will need the FD result, so it is very important to define API to describe the FD result format to user space. And the input about your FD HW result format is certainly helpful to define the API. AFAICS OMAP4 FDIF processes only data stored in memory, thus it seems reasonable to use the videodev interface for passing data to the kernel from user space. But there might be face detection devices that accept data from other H/W modules, e.g. transferred through SoC internal data buses between image processing pipeline blocks. Thus any new interfaces need to be designed with such devices in mind. Also the face detection hardware block might now have an input DMA engine in it, the data could be fed from memory through some other subsystem (e.g. resize/colour converter). Then the driver for that subsystem would implement a video node. I think the direct input image or frame data to FD should be from memory no matter the actual data is from external H/W modules or input DMA because FD will take lot of time to detect faces in one image or frame and FD can't have so much memory to cache several images or frames data. Sorry, I cannot provide much details at the moment, but there exists hardware that reads data from internal SoC buses and even if it uses some sort of cache memory it doesn't necessarily have to be available for the user. Without some hardware background, it is not easy to give a generic FD module design. Still the FD result is associated with an image frame for such H/W, but not necessarily with a memory buffer queued by a user application. For user space application, it doesn't make sense to handle FD results only without image data. Even though there are other ways of input image data to FD, user space still need to know the image data, so it makes sense to associate FD result with a memory buffer. How long it approximately takes to process single image for OMAP4 FDIF ? See the link[2], and my test result is basically consistent
Re: [RFC PATCH v1 5/7] media: v4l2: introduce two IOCTLs for face detection
Hi, On Tue, Dec 6, 2011 at 6:15 AM, Sylwester Nawrocki snj...@gmail.com wrote: On 12/02/2011 04:02 PM, Ming Lei wrote: This patch introduces two new IOCTLs and related data structure defination which will be used by the coming face detection video device. The two IOCTLs and related data structure are used by user space application to retrieve the results of face detection. They can be called after one v4l2_buffer has been ioctl(VIDIOC_DQBUF) and before it will be ioctl(VIDIOC_QBUF). The utility fdif[1] is useing the two IOCTLs to find faces deteced in raw images or video streams. [1],http://kernel.ubuntu.com/git?p=ming/fdif.git;a=shortlog;h=refs/heads/v4l2-fdif Signed-off-by: Ming Lei ming@canonical.com --- drivers/media/video/v4l2-ioctl.c | 38 include/linux/videodev2.h | 70 ++ include/media/v4l2-ioctl.h | 6 +++ 3 files changed, 114 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index e1da8fc..fc6266f 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -2140,6 +2140,30 @@ static long __video_do_ioctl(struct file *file, dbgarg(cmd, index=%d, b-index); break; } + case VIDIOC_G_FD_RESULT: + { + struct v4l2_fd_result *fr = arg; + + if (!ops-vidioc_g_fd_result) + break; + + ret = ops-vidioc_g_fd_result(file, fh, fr); + + dbgarg(cmd, index=%d, fr-buf_index); + break; + } + case VIDIOC_G_FD_COUNT: + { + struct v4l2_fd_count *fc = arg; + + if (!ops-vidioc_g_fd_count) + break; + + ret = ops-vidioc_g_fd_count(file, fh, fc); + + dbgarg(cmd, index=%d, fc-buf_index); + break; + } default: if (!ops-vidioc_default) break; @@ -2234,6 +2258,20 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, } break; } + + case VIDIOC_G_FD_RESULT: { + struct v4l2_fd_result *fr = parg; + + if (fr-face_cnt != 0) { + *user_ptr = (void __user *)fr-fd; + *kernel_ptr = (void *)fr-fd; + *array_size = sizeof(struct v4l2_fd_detection) + * fr-face_cnt; + ret = 1; + } + break; + + } } return ret; diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 4b752d5..073eb4d 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2160,6 +2160,74 @@ struct v4l2_create_buffers { __u32 reserved[8]; }; +/** + * struct v4l2_obj_detection + * @buf_index: entry, index of v4l2_buffer for face detection + * @centerx: return, position in x direction of detected object + * @centery: return, position in y direction of detected object + * @angle: return, angle of detected object + * 0 deg ~ 359 deg, vertical is 0 deg, clockwise + * @sizex: return, size in x direction of detected object + * @sizey: return, size in y direction of detected object + * @confidence: return, confidence level of detection result + * 0: the heighest level, 9: the lowest level Hmm, not a good idea to align a public interface to the capabilities of a single hardware implementation. I think that the current omap interface is general enough, so why can't we use it as public interface? min/max confidence could be queried with relevant controls and here we could remove the line implying range. No, the confidence is used to describe the probability about the correctness of the current detection result. Anyway, no FD can make sure that it is 100% correct. Other HW can normalize its confidence level to 0~9 so that application can handle it easily, IMO. + * @reserved: future extensions + */ +struct v4l2_obj_detection { + __u16 centerx; + __u16 centery; + __u16 angle; + __u16 sizex; + __u16 sizey; How about using struct v4l2_rect in place of centerx/centery, sizex/sizey ? After all it describes a rectangle. We could also use struct v4l2_frmsize_discrete for size but there seems to be missing en equivalent for position, e.g. Maybe user space would like to plot a circle or ellipse over the detected objection, and I am sure that I have seen this kind of plot over detected face before. struct v4l2_position { __s32 x; __s32 y; }; + __u16 confidence; + __u32 reserved[4]; +}; + +#define V4L2_FD_HAS_LEFT_EYE 0x1 +#define V4L2_FD_HAS_RIGHT_EYE 0x2 +#define V4L2_FD_HAS_MOUTH
Re: [RFC PATCH v1 5/7] media: v4l2: introduce two IOCTLs for face detection
On Tue, Dec 6, 2011 at 8:55 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 06 December 2011, Ming Lei wrote: diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 073eb4d..8aeaa1e 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2214,7 +2214,12 @@ struct v4l2_fd_result { __u32 buf_index; __u32 face_cnt; __u32 reserved[6]; - struct v4l2_fd_detection *fd; + + /*make 64/32 compatible*/ + union { + struct v4l2_fd_detection *fd; + __u64 dummy; + }; }; That's not compatible, at least not on any big-endian architecture. If you want to have an indirect pointer, you have to cast it to the __u64 member in user space and back in kernel space. Looks like it is a bit ugly. Using an array added to the end of the v4l2_fd_result structure rather than a pointer would really make this easier IMHO. I have tried to do this, but video_usercopy needs a few changes to handle array args if no indirect pointer is passed to kernel. I am not sure if media guys are happy to accept the changes, :-) thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, Thanks for your review. On Tue, Dec 6, 2011 at 5:55 AM, Sylwester Nawrocki snj...@gmail.com wrote: Hi Ming, (I've pruned the Cc list, leaving just the mailing lists) On 12/02/2011 04:02 PM, Ming Lei wrote: This patch introduces one driver for face detection purpose. The driver is responsible for all v4l2 stuff, buffer management and other general things, and doesn't touch face detection hardware directly. Several interfaces are exported to low level drivers (such as the coming omap4 FD driver)which will communicate with face detection hw module. So the driver will make driving face detection hw modules more easy. I would hold on for a moment on implementing generic face detection module which is based on the V4L2 video device interface. We need to first define an API that would be also usable at sub-device interface level (http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html). If we can define a good/stable enough APIs between kernel and user space, I think the patches can be merged first. For internal kernel APIs, we should allow it to evolve as new hardware comes or new features are to be introduced. I understand the API you mentioned here should belong to kernel internal API, correct me if it is wrong. AFAICS OMAP4 FDIF processes only data stored in memory, thus it seems reasonable to use the videodev interface for passing data to the kernel from user space. But there might be face detection devices that accept data from other H/W modules, e.g. transferred through SoC internal data buses between image processing pipeline blocks. Thus any new interfaces need to be designed with such devices in mind. Also the face detection hardware block might now have an input DMA engine in it, the data could be fed from memory through some other subsystem (e.g. resize/colour converter). Then the driver for that subsystem would implement a video node. I think the direct input image or frame data to FD should be from memory no matter the actual data is from external H/W modules or input DMA because FD will take lot of time to detect faces in one image or frame and FD can't have so much memory to cache several images or frames data. If you have seen this kind of FD hardware design, please let me know. I'm for leaving the buffer handling details for individual drivers and focusing on a standard interface for applications, i.e. new I think leaving buffer handling details in generic FD driver or individual drivers doesn't matter now, since it don't have effect on interfaces between kernel and user space. ioctl(s) and controls. TODO: - implement FD setting interfaces with v4l2 controls or ext controls Signed-off-by: Ming Lei ming@canonical.com --- drivers/media/video/Kconfig | 2 + drivers/media/video/Makefile | 1 + drivers/media/video/fdif/Kconfig | 7 + drivers/media/video/fdif/Makefile | 1 + drivers/media/video/fdif/fdif.c | 645 + drivers/media/video/fdif/fdif.h | 114 +++ 6 files changed, 770 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/fdif/Kconfig create mode 100644 drivers/media/video/fdif/Makefile create mode 100644 drivers/media/video/fdif/fdif.c create mode 100644 drivers/media/video/fdif/fdif.h [...] diff --git a/drivers/media/video/fdif/fdif.h b/drivers/media/video/fdif/fdif.h new file mode 100644 index 000..ae37ab8 --- /dev/null +++ b/drivers/media/video/fdif/fdif.h @@ -0,0 +1,114 @@ +#ifndef _LINUX_FDIF_H +#define _LINUX_FDIF_H + +#include linux/types.h +#include linux/magic.h +#include linux/errno.h +#include linux/kref.h +#include linux/kernel.h +#include linux/videodev2.h +#include media/videobuf2-page.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h +#include media/v4l2-fh.h +#include media/v4l2-event.h +#include media/v4l2-common.h + +#define MAX_FACE_COUNT 40 + +#define FACE_SIZE_20_PIXELS 0 +#define FACE_SIZE_25_PIXELS 1 +#define FACE_SIZE_32_PIXELS 2 +#define FACE_SIZE_40_PIXELS 3 This is still OMAP4 FDIF specific, we need to think about v4l2 controls for this. An ideal would be a menu control type that supports pixel size (width/height), but unfortunately something like this isn't available in v4l2 yet. Yes, it is on TODO list, :-) + +#define FACE_DIR_UP 0 +#define FACE_DIR_RIGHT 1 +#define FACE_DIR_LIFT 2 + +struct fdif_fmt { + char *name; + u32 fourcc; /* v4l2 format id */ + int depth; + int width, height; Could width/height be negative ? I don't think it's the case for pixel resolution. The more proper data type would be u32. Yes, they should be, will fix it in next version. Please refer to struct v4l2_pix_format or struct v4l2_rect. +}; + +struct fdif_setting { + struct fdif_fmt
Re: [RFC PATCH v1 5/7] media: v4l2: introduce two IOCTLs for face detection
On Tue, Dec 6, 2011 at 10:41 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 06 December 2011, Ming Lei wrote: Using an array added to the end of the v4l2_fd_result structure rather than a pointer would really make this easier IMHO. I have tried to do this, but video_usercopy needs a few changes to handle array args if no indirect pointer is passed to kernel. Ah, I see. Or you would have to encode the array size into the ioctl command, which is also ugly in a different way. I am not sure if media guys are happy to accept the changes, :-) Maybe Mauro can comment on which solution he prefers then, given the choice between: 1. adding another handler in drivers/media/video/v4l2-compat-ioctl32.c 2. passing a pointer that is casted to __u64 in user space an back in the kernel 3. extending video_usercopy in some way to make this work, preferably in a generic way. Maybe this one is a good choice, and I think that it is worthy to support the below kind of array parameter: struct v4l2_fd_result { __u32 buf_index; __u32 face_cnt; __u32 reserved[6]; struct v4l2_fd_detection fd[]; }; and it is not difficult to implement it in a generic way so that new array parameters can be supported as 64/32 compatible. 4. using a variable command number like #define VIDIOC_G_FD_RESULT(num) _IOC(_IOC_READ|_IOC_WRITE,'V', 95, \ sizeof(struct v4l2_fd_result) + (num) * sizeof(struct v4l2_fd_detection) 5. requiring the interface to be simplified to return only a single struct v4l2_fd_detection at a time Maybe this one is not user friendly since other v4l2 interfaces provide array parameters way, :-) I agree that none of these are nice. My preferred option would be last one, but I don't know how performance critical the interface is or if it would cause any races that you want to avoid. thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html