Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Mon, Jun 17, 2013 at 4:27 PM, Tony Lindgren 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 Mon, Jun 17, 2013 at 4:06 PM, Arnaud Patard wrote: > Sricharan R 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 Fri, Jun 14, 2013 at 9:31 PM, Ming Lei wrote: > On Thu, Jun 13, 2013 at 6:12 PM, Sricharan R 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 [130612 09:37]: >>>>> * Ming Lei [130603 08:34]: >>>>>> Hi, >>>>>> >>>>>> On Sat, May 18, 2013 at 3:17 AM, Tony Lindgren 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 >> 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 '' > width not an integer constant > drivers/usb/musb/omap2430.c:585:4: error: bit-field '' > 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 mo
Re: [PATCH 2/6] ARM: OMAP2+: Remove board-omap4panda.c
On Thu, Jun 13, 2013 at 6:12 PM, Sricharan R 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 [130612 09:37]: >>>> * Ming Lei [130603 08:34]: >>>>> Hi, >>>>> >>>>> On Sat, May 18, 2013 at 3:17 AM, Tony Lindgren 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 > 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 '' width not an integer constant drivers/usb/musb/omap2430.c:585:4: error: bit-field '' 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 Thu, Jun 13, 2013 at 7:05 PM, Tony Lindgren wrote: > * Sricharan R [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 >> 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 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 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 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 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 wrote: > On 12/03/2012 05:00 AM, Ming Lei wrote: >> On Mon, Dec 3, 2012 at 12:02 AM, Andy Green 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
On Tue, Dec 4, 2012 at 1:09 AM, Alan Stern 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
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
ch- or arch/arm/plat-xxx or arch/arm. I guess it would go in > drivers/usb or drivers/net. How does drivers/usb or drivers/net know the SMSC is used on beagle or panda? Different power control approach is used in the two boards even same SMSC chip is shipped in the two boards. > > Push in ARM-Land is towards single kernels which support many platforms, a > good case in point is omap2plus_defconfg which wants to allow to support all > OMAP2/3/4/5 platforms in one kernel. If you "include" that C file over and > over in board files, it's not very nice for that. They anyway want to > eliminate board files for the single kernel binary reason, and just have one > load of config come in by dtb. I only mean it is reasonable to put the power control code into board file because each board may take different power control approach even same SMSC chip is used. I understand DT only describes the difference of the board via device node, and I am not sure if the current DT is enough to convert the board file into data as far as the problem is concerned. > > So it guides you towards having static helper code once in drivers/ for the > scenarios you support... if that's where you end up smsc is less good a > target for a helper than to have helpers for classes of object like > regulator and clk, that you can combine and reuse on all sorts of target > devices, which is device_asset approach. > > It also guides you to having the special platform sauce be something that > can go into the dtb: per-board code can't. That's why device_asset stuff > only places asset structs in the board file and is removing code from there. > > >>> that perform device_path business out of the omap4panda board file at >>> least. >>> At that point the path matching code becomes generic >>> end-of-the-device-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 Mon, Dec 3, 2012 at 12:37 AM, Andy Green 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
>> Cc: Roger Quadros
>> Cc: Alan Stern
>> Cc: Felipe Balbi
>> Signed-off-by: Ming Lei
>> ---
>> 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
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include
>> #include
>> @@ -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 n
Re: [RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier
On Mon, Dec 3, 2012 at 12:13 AM, Andy Green 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
>> Cc: Roger Quadros
>> Cc: Alan Stern
>> Cc: Felipe Balbi
>> Signed-off-by: Ming Lei
>> ---
>> 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_devi
Re: [RFC PATCH 1/5] Device Power: introduce power controller
On Mon, Dec 3, 2012 at 12:02 AM, Andy Green 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
[RFC PATCH 5/5] usb: omap ehci: remove all regulator control from ehci omap
From: Andy Green
This series migrates it to the hub driver as suggested by
Felipe Balbi.
Cc: Andy Green
Cc: Roger Quadros
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Andy Green
Signed-off-by: Ming Lei
---
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
#include
#include
-#include
#include
#include
#include
@@ -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
[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
Cc: Roger Quadros
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Ming Lei
---
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
#include
#include
+#include
+#include
#include
#include
@@ -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 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
Cc: Roger Quadros
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Ming Lei
---
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
#include
#include
+#include
+#include
#include
#include
@@ -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
+
+/*
+ * 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 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
Cc: Roger Quadros
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Ming Lei
---
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 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"
Cc: Andy Green
Cc: Roger Quadros
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Ming Lei
---
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
+#include
+#include
+#include
+#include
+#include
+
+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
+
+/*
+ * One power controller provides simple power on and power off.
+ *
+ * One power controller can bind to more than one device, which
+ * provides power logically, for example, we can think one usb port
+ * in hub provi
[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=13481835493&r=1&w=2 [2], http://marc.info/?t=13430338453&r=1&w=2 [3], http://marc.info/?l=linux-usb&m=135314427413307&w=2 [4], http://marc.info/?t=13539339473&r=1&w=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
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Thu, Nov 29, 2012 at 12:43 AM, Alan Stern 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 7:06 AM, Ming Lei 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.
+
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Wed, Nov 28, 2012 at 1:55 AM, Andy Green 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
>> 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 roo
Re: [RFC PATCH 1/5] drivers : introduce device_path api
On Wed, Nov 28, 2012 at 12:30 AM, Alan Stern 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 Tue, Nov 27, 2012 at 5:07 AM, Alan Stern 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 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 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] [] (unwind_backtrace+0x0/0xf4) from []
> (__rcu_pending+0x158/0x45c)
> [ 199.203826] [] (__rcu_pending+0x158/0x45c) from []
> (rcu_check_callbacks+0x70/0x1ac)
> [ 199.213653] [] (rcu_check_callbacks+0x70/0x1ac) from
> [] (update_process_times+0x38/0x68)
> [ 199.223968] [] (update_process_times+0x38/0x68) from
> [] (tick_sched_timer+0x88/0xd8)
> [ 199.233917] [] (tick_sched_timer+0x88/0xd8) from []
> (__run_hrtimer+0x7c/0x1e0)
> [ 199.243316] [] (__run_hrtimer+0x7c/0x1e0) from []
> (hrtimer_interrupt+0x108/0x294)
> [ 199.252990] [] (hrtimer_interrupt+0x108/0x294) from []
> (twd_handler+0x34/0x40)
> [ 199.262359] [] (twd_handler+0x34/0x40) from []
> (handle_percpu_devid_irq+0x8c/0x138)
> [ 199.272216] [] (handle_percpu_devid_irq+0x8c/0x138) from
> [] (generic_handle_irq+0x34/0x44)
> [ 199.282714] [] (generic_handle_irq+0x34/0x44) from []
> (handle_IRQ+0x4c/0xac)
> [ 199.291900] [] (handle_IRQ+0x4c/0xac) from []
> (gic_handle_irq+0x2c/0x60)
> [ 199.300781] [] (gic_handle_irq+0x2c/0x60) from []
> (__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] [] (__irq_svc+0x44/0x60) from []
> (__do_softirq+0x64/0x214)
> [ 199.344268] [] (__do_softirq+0x64/0x214) from []
> (irq_exit+0x90/0x98)
> [ 199.352874] [] (irq_exit+0x90/0x98) from []
> (handle_IRQ+0x50/0xac)
> [ 199.361145] [] (handle_IRQ+0x50/0xac) from []
> (gic_handle_irq+0x2c/0x60)
> [ 199.369995] [] (gic_handle_irq+0x2c/0x60) from []
> (__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] [] (__irq_svc+0x44/0x60) from []
> (finish_task_switch+0x4c/0xf0)
> [ 199.418884] [] (finish_task_switch+0x4c/0xf0) from []
> (__schedule+0x410/0x808)
> [ 199.428283] [] (__schedule+0x410/0x808) from []
> (pipe_wait+0x58/0x78)
> [ 199.436859] [] (pipe_wait+0x58/0x78) from []
> (pipe_read+0x454/0x584)
> [ 199.445343] [] (pipe_read+0x454/0x584) from []
> (do_sync_read+0xac/0xf4)
> [ 199.454101] [] (do_sync_read+0xac/0xf4) from []
> (vfs_read+0xac/0x130)
> [ 199.462646] [] (vfs_read+0xac/0x130) from []
> (sys_read+0x40/0x70)
> [ 199.470855] [] (sys_read+0x40/0x70) from []
> (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 6:13 PM, Shilimkar, Santosh 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 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
Re: [PATCH] ARM: OMAP2+: fix gpmc request_irq
On Fri, May 18, 2012 at 3:43 PM, Shilimkar, Santosh 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 3:19 PM, Shilimkar, Santosh 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
[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
Cc: Tony Lindgren
Signed-off-by: Ming Lei
---
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
On Wed, May 16, 2012 at 4:17 PM, Ming Lei wrote: > Jon, > > On Wed, May 16, 2012 at 6:05 AM, Ming Lei wrote: >> >> >> On Tuesday, May 15, 2012, Jon Hunter wrote: >>> Hi Ming, >>> >>> On 05/14/2012 11:53 PM, Ming Lei wrote: >>>> On Thu, May 10, 2012 at 5:35 AM, Jon Hunter wrote: >>>>> From: Jon Hunter >>>>> >>>>> 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 > 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 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
---
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 3/6] arm: omap4: support pmu
On Thu, May 10, 2012 at 5:35 AM, Jon Hunter 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
Re: [PATCH 4/6] ARM: OMAP4: PMU: Add runtime PM support
Jon, On Wed, May 16, 2012 at 6:05 AM, Ming Lei wrote: > > > On Tuesday, May 15, 2012, Jon Hunter wrote: >> Hi Ming, >> >> On 05/14/2012 11:53 PM, Ming Lei wrote: >>> On Thu, May 10, 2012 at 5:35 AM, Jon Hunter wrote: >>>> From: Jon Hunter >>>> >>>> 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 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 4/6] ARM: OMAP4: PMU: Add runtime PM support
On Thu, May 10, 2012 at 5:35 AM, Jon Hunter wrote:
> From: Jon Hunter
>
> 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
> Cc: Will Deacon
> Cc: Benoit Cousson
> Cc: Paul Walmsley
> Cc: Kevin Hilman
>
> Signed-off-by: Jon Hunter
> ---
> 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
> #include
> #include
> +#include
>
> #include
> #include
> @@ -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.res
Re: oprofile and ARM A9 hardware counter
On Wed, May 9, 2012 at 3:51 AM, Jon Hunter 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 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-kernel&m=131946777028358&w=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 wrote: > On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux > 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 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
> 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
> Cc: Benoît Cousson
> Cc: Kevin Hilman
> Cc: Santosh Shilimkar
> Cc: Ming Lei
> ---
> 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 Sun, Apr 1, 2012 at 3:10 AM, Shilimkar, Santosh wrote: > On Sat, Mar 31, 2012 at 2:09 PM, Ming Lei 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 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-kernel&m=133311647410324&w=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 Fri, Mar 30, 2012 at 5:20 PM, Shilimkar, Santosh > >> 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 wrote: > On Mon, Mar 19, 2012 at 17:14:30, Ming Lei wrote: >> On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav 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
Re: [PATCH] ARM: OMAP3+: fix oops triggered in omap_prcm_register_chain_handler
On Wed, Mar 21, 2012 at 10:13 PM, Kevin Hilman wrote: > Ming Lei 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(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 : []lr : []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] [] (irq_setup_generic_chip+0x6a/0x84) from
[] (omap_prcm_register_chain_handler+0x147/0x1a0)
[1.948516] [] (omap_prcm_register_chain_handler+0x147/0x1a0) from
[] (do_one_initcall+0x65/0xf4)
[1.959500] [] (do_one_initcall+0x65/0xf4) from []
(kernel_init+0xab/0x138)
[1.968529] [] (kernel_init+0xab/0x138) from []
(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
Acked-by: Tero Kristo
Signed-off-by: Ming Lei
---
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
[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 : []lr : []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] [] (irq_setup_generic_chip+0x6a/0x84) from
[] (omap_prcm_register_chain_handler+0x147/0x1a0)
[1.948516] [] (omap_prcm_register_chain_handler+0x147/0x1a0) from
[] (do_one_initcall+0x65/0xf4)
[1.959500] [] (do_one_initcall+0x65/0xf4) from []
(kernel_init+0xab/0x138)
[1.968529] [] (kernel_init+0xab/0x138) from []
(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
Signed-off-by: Ming Lei
---
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 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 wrote: > Vaibhav Hiremath 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 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 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 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 > > 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)
as:/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
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 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: [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 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
[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 --- 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: Minimum timing resolution in Ubuntu/Linaro on the PandaBoard ES
Hi,
On Wed, Feb 8, 2012 at 1:21 PM, Dmitry Antipov
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
On Mon, Jan 30, 2012 at 9:43 PM, stephane eranian 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 Mon, Jan 30, 2012 at 1:36 AM, stephane eranian
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
Hi, 2012/1/27 Will Deacon : > Mans, > > On Fri, Jan 27, 2012 at 12:56:35PM +, Måns Rullgård wrote: >> Will Deacon 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
Hi, On Fri, Jan 27, 2012 at 8:13 PM, Will Deacon 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-kernel&m=132697975416659&w=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
On Fri, Jan 20, 2012 at 9:47 PM, stephane eranian 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-kernel&m=132697975416659&w=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 9:32 PM, stephane eranian
wrote:
> On Thu, Jan 19, 2012 at 2:26 PM, Ming Lei wrote:
>> Hi,
>>
>> On Thu, Jan 19, 2012 at 9:14 PM, Ming Lei wrote:
>>> On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian
>>> wrote:
>>>> On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian
>>>>> 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,
On Thu, Jan 19, 2012 at 9:14 PM, Ming Lei wrote:
> On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian
> wrote:
>> On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei wrote:
>>> Hi,
>>>
>>> On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian
>>> 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
On Thu, Jan 19, 2012 at 8:51 PM, stephane eranian wrote: > On Thu, Jan 19, 2012 at 1:45 PM, Ming Lei wrote: >> Hi, >> >> On Thu, Jan 19, 2012 at 7:34 PM, stephane eranian >> 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 7:34 PM, stephane eranian 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 > > # 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 wrote: >> Hi, >> >> On Thu, Jan 19, 2012 at 5:58 AM, stephane eranian >> 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 wrote: >>>> Hi, >>>> >>>> On Wed, Jan 18, 2012 at 5:54 PM, stephane eranian >>>>> 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
Hi, On Thu, Jan 19, 2012 at 5:58 AM, stephane eranian 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 wrote: >> Hi, >> >> On Wed, Jan 18, 2012 at 5:54 PM, stephane eranian >>> 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
On Wed, Jan 18, 2012 at 7:39 PM, Shilimkar, Santosh
wrote:
> On Wed, Jan 18, 2012 at 10:33 AM, Ming Lei wrote:
>> Hi Will and stephane,
>>
>> On Wed, Jan 18, 2012 at 12:18 PM, Ming Lei wrote:
>>> Hi stephane & Will,
>>>
>>> On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian
>>> 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
>> 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 Wed, Jan 18, 2012 at 5:54 PM, stephane eranian > 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
Hi Will and stephane,
On Wed, Jan 18, 2012 at 12:18 PM, Ming Lei wrote:
> Hi stephane & Will,
>
> On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian
> 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
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 stephane & Will, On Tue, Jan 10, 2012 at 8:46 AM, stephane eranian 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-omap&m=132162118104901&w=2 [3],http://marc.info/?t=13222762152&r=1&w=2 [4],http://marc.info/?t=13222762172&r=1&w=2 [5],http://marc.info/?t=13222762173&r=1&w=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 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-kernel&m=131946761428296&w=2 > > However, as Stephane has pointed out here: > > http://marc.info/?l=linux-omap&m=132585784125352&w=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 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 >> --- >> 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, In
Re: [QUESTION] How to set static/dynamic dependency between clock domains
On Fri, Dec 23, 2011 at 4:45 PM, Shilimkar, Santosh
wrote:
> On Fri, Dec 23, 2011 at 2:00 PM, Ming Lei wrote:
>> Hi Shilimkar,
>>
>> Sorry for the delay.
>>
>> On Mon, Dec 19, 2011 at 2:17 PM, Shilimkar, Santosh
>> 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
> 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
> Acked-by: Benoit Cousson
Please feel free to add:
Reported-and-tested-by: Ming Lei
> ---
> 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
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
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 Sat, Dec 17, 2011 at 12:09 AM, Shilimkar, Santosh 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-kernel&m=132387140703874&w=2 [2], http://marc.info/?l=linux-kernel&m=132387155403929&w=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: [QUESTION] How to set static/dynamic dependency between clock domains
Hi, On Fri, Dec 16, 2011 at 9:24 PM, Ming Lei 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 Fri, Dec 16, 2011 at 11:13 PM, Shilimkar, Santosh wrote: > On Fri, Dec 16, 2011 at 7:20 PM, Ming Lei wrote: >> Hi, >> >> On Fri, Dec 16, 2011 at 9:24 PM, Ming Lei 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 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 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");
[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
[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
---
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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "odif.h"
+#include
+#include
+#include
+
+#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*work_mem_addr;
+ dma_addr_t work_dma;
+ dma_addr_t pict_dma;
+ unsigned long pict_mem_len;
+
+ struct odif_buffer *pend
[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
---
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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#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->ref);
+ return v4l2_fh_open(file);
+}
+
+static unsigned int
+odif_poll(struct file *file, struct poll
[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
Signed-off-by: Ming Lei
---
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 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
---
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
+ *
+ * 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
+#include
+#include
+
+#include
+#include
+
+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);
+
[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
---
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 2/8] omap4: build fdif omap device from hwmod
Signed-off-by: Ming Lei
---
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 1/8] omap4: introduce fdif(face detect module) hwmod
Signed-off-by: Ming Lei
---
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 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
---
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_
[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
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
On Tue, Dec 13, 2011 at 1:59 PM, HeungJun, Kim 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 > 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?
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 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 >> >> >> 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,
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 wrote:
> Signed-off-by: Ming Lei
> ---
> 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 1:43 AM, Sylwester Nawrocki >> 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 6/7] media: video: introduce face detection driver module
On Fri, Dec 9, 2011 at 7:25 AM, Sylwester Nawrocki 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. >&
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 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 t
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 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
>> ---
>> 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.
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 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 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 necessaril
Re: [RFC PATCH v1 5/7] media: v4l2: introduce two IOCTLs for face detection
Hi,
On Tue, Dec 6, 2011 at 10:52 PM, Ming Lei wrote:
> On Tue, Dec 6, 2011 at 10:41 PM, Arnd Bergmann wrote:
>> 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.
How about the blow patch to support 64/32 compatible array parameter?
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index e1da8fc..72c81f7 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -2239,6 +2239,26 @@ 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)
+{
+ int ret = 0;
+
+ switch (cmd) {
+ case VIDIOC_G_FD_RESULT: {
+ struct v4l2_fd_result *fr = parg;
+
+ *extra_len = fr->faces_cnt *
+ sizeof(struct v4l2_fd_detection);
+ ret = 1;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return ret;
+}
+
long
video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
v4l2_kioctl func)
@@ -2251,6 +2271,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 +2301,29 @@ 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 out_array_args;
+ old_mbuf = mbuf;
+ size = extra + _IOC_SIZE(cmd);
+ mbuf = kmalloc(size, GFP_KERNEL);
+ if (NULL == mbuf)
+ return -ENOMEM;
+ 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;
+out_array_args:
has_array_args = err;
if (has_array_args) {
@@ -2321,7 +2362,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;
}
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 5/7] media: v4l2: introduce two IOCTLs for face detection
On Tue, Dec 6, 2011 at 10:41 PM, Arnd Bergmann 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
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 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 >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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
