RE: Boot hang regression 3.10.0-rc4 - 3.10.0
On Thu, Jul 04, 2013 at 21:30:14, Mark Jackson wrote: On 04/07/13 16:14, Mark Jackson wrote: On 04/07/13 14:25, Mark Jackson wrote: Our custom AM335x board has been booting just fine under 3.10.0-rc4. I've just done a git pull to update to 3.10 (now that it's released) and the board now hangs. Before I start trying to bisect the issue, does anyone have an clues ? Okay ... I've now bisected it to:- a630fbfbb1beeffc5bbe542a7986bf2068874633 is the first bad commit commit a630fbfbb1beeffc5bbe542a7986bf2068874633 Author: Tony Lindgren t...@atomide.com Date: Mon Jun 10 07:39:09 2013 -0700 serial: omap: Fix device tree based PM runtime However, reverting the commit does *not* fix the issue, weird !! But I have now tested:- v3.10-rc4 - works v3.10-rc5 - works v3.10-rc6 - works v3.10-rc7 - works v3.10 - works origin/master - hangs Any ideas ? I just checked the behavior on my AM335x-EVM. Current mainline boots fine provided I don't use earlyprintk. The offending patch [1] in this case is the one that tries to get rid of omap_serial_early_init() for DT boot. This change inadvertently also results in the console UART getting reset and idled during bootup and that's where the boot stops for you. I think if you skip earlyprintk from the bootargs you should see the system booting fine. I guess we need to retain the NO_IDLE and NO_RESET aspect for the console UART in omap_serial_early_init() to get earlyprintk working again. Regards, Vaibhav B. [1] http://www.spinics.net/lists/linux-omap/msg91825.html
RE: Boot hang regression 3.10.0-rc4 - 3.10.0
Hi Tony, On Fri, Jul 05, 2013 at 17:29:59, Tony Lindgren wrote: * Bedia, Vaibhav vaibhav.be...@ti.com [130705 01:17]: I just checked the behavior on my AM335x-EVM. Current mainline boots fine provided I don't use earlyprintk. The offending patch [1] in this case is the one that tries to get rid of omap_serial_early_init() for DT boot. This change inadvertently also results in the console UART getting reset and idled during bootup and that's where the boot stops for you. I think if you skip earlyprintk from the bootargs you should see the system booting fine. I guess we need to retain the NO_IDLE and NO_RESET aspect for the console UART in omap_serial_early_init() to get earlyprintk working again. Hmm nothing should get idled while earlyprintk is running, and then when the serial driver kicks in it should not idle anything by default. And for DT based booting we should not have mach-omap2/serial.c initialize anything. If I add in the HWMOD flags without any reverts I get to the point where the serial driver comes up but the boot eventually stops [1]. Without the flags the boot stops much earlier [2] just like Mark reported. I wonder if this is because the timeouts get now initialized to 0 instead of -1 for the serial driver? You meant initialized to -1, right? There's an additional check for timeout being 0. Unless i am missing something DT-boot will start off with timeout set to 0 and then get forced to -1. Regards, Vaibhav === [1] [0.00] Booting Linux on physical CPU 0x0 ... [0.190107] SMP: Total of 1 processors activated (363.72 BogoMIPS). [0.196711] CPU: All CPU(s) started in SVC mode. [0.205580] devtmpfs: initialized stops here if booted with earlyprintk [2] [0.00] Booting Linux on physical CPU 0x0 ... [1.453364] pinctrl-single 44e10800.pinmux: 142 pins at pa f9e10800 size 568 [1.467121] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled stops here if booted with earlyprintk -- 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: Boot hang regression 3.10.0-rc4 - 3.10.0
On Fri, Jul 05, 2013 at 18:50:10, Bedia, Vaibhav wrote: Hi Tony, On Fri, Jul 05, 2013 at 17:29:59, Tony Lindgren wrote: * Bedia, Vaibhav vaibhav.be...@ti.com [130705 01:17]: I just checked the behavior on my AM335x-EVM. Current mainline boots fine provided I don't use earlyprintk. The offending patch [1] in this case is the one that tries to get rid of omap_serial_early_init() for DT boot. This change inadvertently also results in the console UART getting reset and idled during bootup and that's where the boot stops for you. I think if you skip earlyprintk from the bootargs you should see the system booting fine. I guess we need to retain the NO_IDLE and NO_RESET aspect for the console UART in omap_serial_early_init() to get earlyprintk working again. Hmm nothing should get idled while earlyprintk is running, and then when the serial driver kicks in it should not idle anything by default. And for DT based booting we should not have mach-omap2/serial.c initialize anything. If I add in the HWMOD flags without any reverts I get to the point where the serial driver comes up but the boot eventually stops [1]. Without the flags the boot stops much earlier [2] just like Mark reported. Err.. the log with HWMOD flags added is [2] and without flags is [1]. Sorry for the confusion. I wonder if this is because the timeouts get now initialized to 0 instead of -1 for the serial driver? You meant initialized to -1, right? There's an additional check for timeout being 0. Unless i am missing something DT-boot will start off with timeout set to 0 and then get forced to -1. Regards, Vaibhav === [1] [0.00] Booting Linux on physical CPU 0x0 ... [0.190107] SMP: Total of 1 processors activated (363.72 BogoMIPS). [0.196711] CPU: All CPU(s) started in SVC mode. [0.205580] devtmpfs: initialized stops here if booted with earlyprintk [2] [0.00] Booting Linux on physical CPU 0x0 ... [1.453364] pinctrl-single 44e10800.pinmux: 142 pins at pa f9e10800 size 568 [1.467121] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled stops here if booted with earlyprintk -- 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: [RFC PATCH 0/2] cpufreq/regulator: Limit minimum voltage only
(removing Anil's email-id since it's no longer valid) On Sat, Apr 20, 2013 at 05:54:10, Kondratiuk, Taras wrote: On 04/19/2013 07:21 PM, Nishanth Menon wrote: On 14:55-20130419, Taras Kondratiuk wrote: Using a voltage tolerance for doing DVFS is not a proper way. It leads to a few issues: - voltage is limited to a narrow range near OPP voltage, so other consumers of the same regulator can't set their own constraints if they don't overlap with this narrow range. No ganged rails :( - usually OPP voltage is an absolute minimum voltage necessary for correct work (not taking into account AVS). The absolute minimum voltage part is not applicable for all SoCs. In case of AM335x there a nominal voltage that is specified in the OPP table and there's a plus/minus tolerance at the IO level within which things are guaranteed to work. To lower the power consumption we would want to go as low as possible without violating the requirement at the SoC boundary level. I don't know how the OPP voltages are speced for non TI devices but if there's a permissible range I guess everyone would like to set the voltage near the lower end of the spectrum. Applying plus/minus tolerance can lead to an unstable device. For example omap-cpufreq has 4% tolerance configured, so for OMAP4430 MPU OPP50 we get 0.984V instead of 1.025V. So if you set the tolerance to 0% instead of removing it completely won't the problem go away? there is a reason for this - board level IRDROP and the real PMIC accuracy compared to the SoC assumption about the PMIC accuracy. I don't see how current implementation of voltage-tolerance can help with this. It does help (more below). That said, I had been always been a little confused with the usage in AM335x. For that matter, I dont even think this is constrainted to TI SoC usage, other SoCs also probably have the same pain. How does it actually work? I think this is a question to Afzal as he is an original author of commit 42daffd2d6c665716d442d518022ecaad17ddf64 which later migrated to cpufreq-cpu0 driver. I can only guess... Without tolerance cpufreq requests the same value for min_uV and max_uV. So regulator have to set an exact voltage value on the rail, which is not always possible if different PMICs can be used for the SoC. For example in v3.9-rc7 voltage-tolerance is used *only* in AM33xx which can use two PMICs: TPS65217 and TPS65910. These PMICs have different voltage steps so they can't set the same voltage. I think this was the reason for adding voltage-tolerance. The PMICs have the same step size of 12.5mV but unfortunately they don't have a register setting to meet the nominal voltage requirement for all the OPPs. As per the SoC datasheet, for OPP120 the nominal voltage is 1.26V but the closest that the PMIC outputs can come is either 1.25V or 1.275V. Now i think there's been some confusion in the implementation phase due to things like board level IR drops and variations mentioned in the PMIC datasheets which has resulted in the tolerance being used both in am33xx.dtsi and then again in the cpufreq driver. Ignoring the PMIC variations and board level IR drops for moment, the way I think it should have been done is - OPP entries have the nominal voltages and we specify the tolerance either in % in the DT file. The cpufreq driver looks up the nominal voltage from the OPP table and then requests the regulator framework to factor in the tolerance and then select the lowest permissible voltage. *But* there is a trick. If you compare MPU OPP voltages in AM335x datasheet and am33xx.dtsi you will see that am33xx.dtsi has already applied tolerance (2%) on top of nominal voltages. So final range passed to regulator is [Vnom; Vnom+2*tol] instead of [Vnom-tol; Vnom+tol]. That's why it works for AM335x, but IMHO it is a hack. I agree. It sort of ended up a hack that needs to be fixed by removing the additional tolerance in am33xx.dtsi. That patch broke omap-cpufreq functionality, since mach-omap2/opp_data.c files doesn't have tolerance added on top of nominal voltages. Again, if you specify 0% as the tolerance this would be fine, no? regulator_set_voltage_min() should solve AM335x issue without introducing voltage-tolerance hack. No, we want to pass on all the factors to be considered to the appropriate framework which in this case is the regulator framework and let it decide the min voltage. If you consider multiple users like ABB and AVS, I think it makes much more sense to have the requirements from the different users being passed on to the regulator framework and letting it decide what works for all of them. Probably I need to add one more patch to the series which will remove voltage-tolerance from am335x.dtsi and set CPU voltages back to nominal. Lets say ntarget/Vnom has PMIC tolerance of 5% (SoC tolerance assumed for OPP voltage documented in data manual for the SoC), say the
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Kevin, On Thu, Apr 11, 2013 at 19:45:33, Kevin Hilman wrote: Kevin Hilman khil...@linaro.org writes: Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Regards, Vaibhav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: Hi, On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: Hi Kevin, On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddarsourav.pod...@ti.com Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '-is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's -prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's -complete callback. Kevin I was working on your above suggestions, but realised there is not only console uart which has the requirement of keeping the clocks enabled while going on suspend. If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has no_idle_on_suspend property used. Can you please ask the AM33xx folks how (and why) this is being used? I don't see/find a driver for this device in mainline, so without a driver this flag will not be used. Looping in Vaibhav Bedia for ocmcram.. [Vaibhav]: There is a discussion going on about a cleaner way of handling ti, no_idle_on_suspend part (as this is a sort of hack). We got a way around for UART ($subject) by making serial core/driver handle this for us. But with this, we will delete codes around no_idle_on_suspend flag in omap_device file. But, we realised that its not only UART which requires the clocks to be active whie going for suspend. There is a dts entry for ocmcram also. As Kevin also pointed out, we don't see a driver for this device in mainline, It would be great if you can explain how its getting used? You can find the complete discussion on v3 here: https://lkml.org/lkml/2013/4/5/239 The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. From the changelog which added this flag: Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag. We had discussed about the usage of this flag in the RFC version of the patch [1]. Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. Regards, Vaibhav -- 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 v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Daniel, On Wed, Apr 03, 2013 at 17:22:41, Daniel Mack wrote: Hi Vaibhav, On Mon, Dec 31, 2012 at 2:07 PM, Vaibhav Bedia vaibhav.be...@ti.com wrote: AM335x supports various low power modes as documented in section 8.1.4.3 of the AM335x TRM which is available @ http://www.ti.com/litv/pdf/spruh73f May I ask about the plans for this series? Will you be re-spinning them for a current tree, and what's the planned merge window for it? I am tied up in some other stuff right now. I plan to address Kevin's comments and repost the patches soon. Since rc5 is already out i think v3.10 is not a realistic target any more but I do hope to have this accepted for v3.11. Regards, Vaibhav -- 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+: am33xx: preserve JTAG clock aka debugss_ick.
On Thu, Mar 07, 2013 at 18:43:27, Andreas Fenkart wrote: This fixes JTAG support on am33xx. Please refer to http://www.spinics.net/lists/linux-omap/msg87476.html Regards, Vaibhav -- 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 v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
On Mon, Feb 18, 2013 at 21:41:49, Kevin Hilman wrote: [...] By default these IPs don't have MSTANDBY asserted. When you say by default, I guess you mean after reset (and/or context loss), right? Yes When a low power transition happens, the peripheral power domain loses context and hence the forced MSTANDBY configuration in the IP is lost. To work around this problem we need to assert MSTANDBY in every suspend-resume iteration. Yuck. More clever hardware. ;) We are getting this gradually :) I agree that this might hide PM bugs in the driver but to solve this problem we need some mechanism for the PM code to know whether or not a driver is bound to the corresponding platform devices. Any suggestions on this? Driver bound status can be tracked easily using bus notifiers. You can see an example in the omap_device core. Ok. I'll try to use the driver bound status in the next version. [...] +/* + * GFX_L4LS clock domain needs to be woken up to + * ensure thet L4LS clock domain does not get stuck in transition + * If that happens L3 module does not get disabled, thereby leading + * to PER power domain transition failing + * + * The clock framework should take care of ensuring + * that the clock domain is in the right state when + * GFX driver is active. Are you suggesting that the clock framework is not doing this already? No. This clkdm_*() calls are here to work-around an issue that I observed when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls clock domain across every suspend-resume is something I don't think can be pushed to the clock framework. I still don't follow what you're suggesting the clock framework should do. Are you describing the case when there is a GFX driver vs. when there isn't a driver? If so, it needs to be more clear. No. The issue with GFX_L4LS happens irrespective of the state of the GFX driver and needs to be handled in the suspend-resume sequence. I guess the second part of comment is what created the confusion. I'll get rid of that. Also, some more description about why the device gets 'stuck in transition' would be helpful. Is this an erratum workaround? I'll follow up with the design folks to find out more. From some past discussions this is not expected so looks like we need to an erratum published for this issue. I see, then probably a TODO here with more description would be more helpful. So, IIUC, without implemeting this, the drivers will never be able to detect loss of context, correct? Sounds like something that should be decribed in the changelog as that's a rather important limitation to this implementaion. Ok. I'll address this limitation in the next version and improve the changelog. + +/* Give some time to M3 to respond. 500msec is a random value here */ random? really? Sort of. I don't have any numbers from the h/w guys on the worst case delay in getting an interrupt from M3 to MPU. At the same time I want to handle the scenario where something goes wrong on the M3 side and it doesn't respond. OK, then it's not random. You have some reasoning behind the number that should be documented. Will do. That being said, in the absence of numbers from HW folks, can't you measure the typical times so you know roughly what's normal. I'll do some timer based profiling and get rough numbers for this. [...] Why not omap_device_enable(pdev) ? The objective is to leverage the hwmod code to get the WKUP-M3 functional and not have OMAP runtime PM code coming in the way. FWIW, it is not runtime PM getting in the way. Using omap_device_enable() triggers the following dev_warn() from omap_device_enable(). Looking closer at the trace, you'll see it's not omap_device_enable() that is triggering this warning. What is happening is that omap_device has a late_initcall which forcibly idles omap_devices that have been enabled, but that don't have a driver. You're hacking around that. Thanks for the explanation. I should have looked closer :( IMO, this would be a much cleaner implementation if you just created a small driver for the wkup_m3. You're already doing a bunch of driver-like stuff for it (requesting base/IRQs, mapping regions, firmware, etc.) I think this should separated out into a real driver. Then it will be bound to the right omap_device, and normal PM operations will work as expected. Also, doing it this way will be more flexible for those wanting to use their own firmware on the M3, or customize the current firmware. Hmm... that definitely sounds more flexible. It should also help in the next SoC AM437x which has a similar PM architecture. Where would you suggest placing this minimal driver? Regards, Vaibhav -- To unsubscribe from this list: send the line
RE: AM33xx's PWM Time Base Clock Enable signals
+Avinash On Wed, Feb 20, 2013 at 18:06:24, Balbi, Felipe wrote: Hi Paul, Tony, how do you guys want to handle PWM's Time Base clocks which are enabled via control module ? They're controlled via offset 0x664 (pwmss_ctrl). Page 793 of AM33xx's TRM has more information: Avinash has already posted the patch for this https://lkml.org/lkml/2013/2/12/43 Regards, Vaibhav -- 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/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
On Sat, Feb 16, 2013 at 11:18:36, Shilimkar, Santosh wrote: [...] For the duplicate ioremapping, I don't think there's any need to do it if we get things right. Note that if the ioremap matches a static map area there is no cost to ioremap it multiple times. Thats true though now on OMAP we removed most of the static mappings. The main issue is waste of IO space because, we end up mapping same area two times for all the OMAP drivers. This can be optimized with a arch ioremap caller hook but as discussed here, its nice to have rather than something important. Err.. I was looking at the iotable_init for OMAPx in mainline and it looks like most (all?) of the peripherals are already covered in the static mappings? Regards, Vaibhav -- 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/RFT PATCH 2/2] SERIAL: OMAP: Remove the idle handling from the driver
Hi, On Fri, Feb 15, 2013 at 19:13:42, Shilimkar, Santosh wrote: On Friday 15 February 2013 07:04 PM, Felipe Balbi wrote: Hi, On Fri, Feb 15, 2013 at 06:49:20PM +0530, Santosh Shilimkar wrote: @@ -279,8 +259,6 @@ static void serial_omap_stop_tx(struct uart_port *port) serial_out(up, UART_IER, up-ier); } -serial_omap_set_forceidle(up); - pm_runtime_mark_last_busy(up-dev); pm_runtime_put_autosuspend(up-dev); } @@ -348,7 +326,6 @@ static void serial_omap_start_tx(struct uart_port *port) pm_runtime_get_sync(up-dev); serial_omap_enable_ier_thri(up); -serial_omap_set_noidle(up); this patch is changing behavior. Currently driver has: start_tx() get_sync() set_noidle() put_autosuspend() stop_tx() get_sync() set_force_idle() put_autosuspend() with this change, you will have: start_tx() get_sync() set_noidle() put_autosuspend() set_force_idle() this in itself might be enough to go back to corrupted serial if put_autosuspend is so quick that set_force_idle() is called before all data has been shifted out of FIFO and through the UART lines. Really. Infact the old sequence was buggy because you are setting force_idle even before suspending the device. And that force idle then that bug has to be fixed first and patch needs to be sent to stable before we change that part of the code, wouldn't you agree ? Agree. Will be good to get that fixed for stable. Probably Sourabh can fix that one. isn't really force idle but setting ip to smart idle. Just look at what serial.c indeed. Before doing this, you should at least test that removing pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend() from start_tx() and removing pm_runtime_get_sync() from stop_tx() works fine. Seems to work for me with above changes as well. Can you please try out and see if you see any issue. I doubt you will... what I'm saying is that current code, as you put it yourself, is buggy, so we ought to fix the bugs first before changing behavior. If not for anything else, at least to have a clean tree which we can bisect. Also, $SUBJECT isn't improving the situation regarding UART Wakeup, there is still the regression of UART never being wakeup capable. You are mixing the stuff here. The subject is correct. -enable_wakeup() sets the IP to smart_idle_wakeup, which is done on SYSC register. If $SUBJECT wants to fix SYSC usage, it ought to fix them all. But SYSC is already in smart_idle_wakeup() mode. I get your point though. The main purpose of that wakeup hook is to trigger io_ring and pad wakeup. BTW, Rajendra is looking at how to get rid of wakeup function pointer as well since that also comes in way for DT. With these changes the async wakeup mechanism for UART which depends on SIDLE bits being set to 0x3 and ENWAKEUP being set to 0x1 breaks. I noticed this while testing these changes on top of the AM335x suspend-resume support. How about leveraging the generic wakeup flag for all devices to get the required functionality for wakeup? A call to device_init_wakeup() in probe, followed by a check for device_may_wakeup() in the driver's suspend routine can be used to have omap_device_idle_hwmods() configure the SIDLE bits appropriately. This should help configure SIDLE to FORCE/NO_IDLE during active mode along with the appropriate SYSC configuration during suspend. The device_may_wakeup() check could even be used to enable IO Daisy chaining feature for the IOs. Regards, Vaibhav -- 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/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
On Fri, Feb 15, 2013 at 12:23:08, Balbi, Felipe wrote:
On Thu, Feb 14, 2013 at 02:22:47PM -0800, Tony Lindgren wrote:
* Paul Walmsley p...@pwsan.com [130214 12:51]:
Hi,
On Thu, 14 Feb 2013, Tony Lindgren wrote:
I don't think so as hwmod should only touch the sysconfig space
when no driver has claimed it.
hwmod does need to touch the SYSCONFIG register during pm_runtime suspend
and resume operations, and during device enable and disable operations.
Here's the relevant code:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195
But that's triggered by runtime PM from the device driver, right?
I think it's fine for hwmod to do that as requested by the device
driver via runtime PM since hwmod is the bus glue making the drivers arch
independent.
I think the only piece we're missing for that is for driver to run
something like pm_runtime_init() that initializes the address space
to hwmod. Or just bus specific ioremap like you're suggesting later
on.
Just to recap what we've discussed earlier, the reasons why we want
reset and idle functions should be in the driver specific header are:
1. There's often driver specific logic needed in addition to the
syconfig tinkering in the reset/idle functions.
how about introducing generic device_reset() and device_idle() hooks
which driver can implement and can be called by bus glue ?
Something like:
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 03d7bb1..9c921e2 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -256,6 +256,8 @@ typedef struct pm_message {
* these conditions and handle the device as appropriate, possibly queueing
* a suspend request for it. The return value is ignored by the PM core.
*
+ * @runtime_reset: Resets the device and puts it in a known state.
+ *
* Refer to Documentation/power/runtime_pm.txt for more information about the
* role of the above callbacks in device runtime power management.
*
@@ -285,6 +287,7 @@ struct dev_pm_ops {
int (*runtime_suspend)(struct device *dev);
int (*runtime_resume)(struct device *dev);
int (*runtime_idle)(struct device *dev);
+ int (*runtime_reset)(struct device *dev);
};
I am not a runtime PM expert but runtime_reset() looks a good option to me.
I expect most of the drivers won't need to do anything different from what
the hwmod code already does. IPs which have special reset requirements
can either extend the defaults ops or just override it. On AM33xx there
are some special requirements for CPSW, DCAN, PWMSS reset handling but
AFAIK none of the other IPs need to do anything special and we don't want
to duplicate the reset code in all of them.
Regards,
Vaibhav
--
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/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Fri, Feb 15, 2013 at 12:14:29, Balbi, Felipe wrote: Hi, On Thu, Feb 14, 2013 at 08:47:53PM +, Paul Walmsley wrote: Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 Drivers shouldn't touch their IP block's SYSCONFIG registers. They don't why not ? It's part of the driver's address space anyway. It's not part of the IP in question (usb, ethernet, etc) but it's part of the TI wrapper which usually involves a bridge (ocp2scp, ocp2axi, etc) plus the TI-specific integration registers (revision, sysc, syss...). So they're not part of the licensed IP, but they're part of the TI wrapper around those. That's all the more reason to not allow the drivers to touch the SYSCONFIG register. We have IPs like EDMA, PWMSS, McASP etc which are common to Davinci and OMAP. The integration approach was different in the past and now if we want the same driver to work on both the platforms we have to keep the code for taking care of the integration details out of the drivers. IMO omap_hwmod does an excellent job of taking care of all or rather most of the IP integration idiosyncrasies. I really don't understand why people make a big fuss about hwmod. With the right SoC data things just work. Most of the driver authors don't take the pains to understand how the SoC PM gets impacted by toggling a few bits in SYSCONFIG and hence it's best to abstract away all the critical pieces out of drivers. For specific cases like custom reset handling I think it make much more sense to extend runtime PM that already build upon the hwmod code for OMAP. The drivers shouldn't have to worry about the integration details. Trying to shove a common piece of code into all the drivers is equivalent to taking a huge step backwards in the ongoing consolidation not just across OMAP and AMxx parts but also across Davinci and OMAP. Regards, Vaibhav -- 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 v2 07/18] ARM: OMAP2+: AM33XX: hwmod: Update TPTC0 hwmod with the right flags
Hi Kevin, On Tue, Feb 12, 2013 at 05:03:23, Kevin Hilman wrote: Vaibhav Bedia vaibhav.be...@ti.com writes: TPTC0 needs to be idled and put to standby under SW control. Please elaborate about why (e.g. HW support not available, HW support broken/buggy, etc.) since these blocks are not well documented in the docs that I have (spruh73f.) This change was picked up for v3.9. I'll submit a follow-up patch adding in a comment in the hwmod file for this. Regards, Vaibhav -- 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 v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Kevin,
On Tue, Feb 12, 2013 at 06:57:50, Kevin Hilman wrote:
[...]
+
+void (*am33xx_do_wfi_sram)(void);
static?
Will fix.
[...]
+
+ /*
+* By default the following IPs do not have MSTANDBY asserted
+* which is necessary for PER domain transition. If the drivers
+* are not compiled into the kernel HWMOD code will not change the
+* state of the IPs if the IP was not never enabled. To ensure
+* that there no issues with or without the drivers being compiled
+* in the kernel, we forcefully put these IPs to idle.
+*/
+ omap_hwmod_enable(usb_oh);
+ omap_hwmod_enable(tptc0_oh);
+ omap_hwmod_enable(tptc1_oh);
+ omap_hwmod_enable(tptc2_oh);
+ omap_hwmod_enable(cpsw_oh);
+
+ omap_hwmod_idle(usb_oh);
+ omap_hwmod_idle(tptc0_oh);
+ omap_hwmod_idle(tptc1_oh);
+ omap_hwmod_idle(tptc2_oh);
+ omap_hwmod_idle(cpsw_oh);
I think I asked this in my review of v1, but why does this need to
happen on every suspend attempt?
This should happen once on init, which will handle the case where there
are no drivers, and if there are drivers, then the drivers need to
handle this properly.
I don't like this happening here on every suspend attempt, because it
will surely hide bugs where drivers are not properly managing their own PM.
By default these IPs don't have MSTANDBY asserted. When a low power transition
happens, the peripheral power domain loses context and hence the forced
MSTANDBY configuration in the IP is lost. To work around this problem we need
to assert MSTANDBY in every suspend-resume iteration.
I agree that this might hide PM bugs in the driver but to solve this problem we
need some mechanism for the PM code to know whether or not a driver is bound
to the corresponding platform devices. Any suggestions on this?
+ /* Try to put GFX to sleep */
+ pwrdm_set_next_fpwrst(gfx_pwrdm, PWRDM_FUNC_PWRST_OFF);
+
+ ret = cpu_suspend(0, am33xx_do_sram_idle);
+ status = pwrdm_read_fpwrst(gfx_pwrdm);
+ if (status != PWRDM_FUNC_PWRST_OFF)
+ pr_err(GFX domain did not transition\n);
+ else
+ pr_info(GFX domain entered low power state\n);
Do you really want this printed every time?
Hmm... it could perhaps be clubbed with the overall status that's
printed. I kept it here since the GFX power domain is completely
under MPU control and hence this information would be useful in
finding out if there's a problem with the GFX suspend-resume.
+ /*
+* GFX_L4LS clock domain needs to be woken up to
+* ensure thet L4LS clock domain does not get stuck in transition
+* If that happens L3 module does not get disabled, thereby leading
+* to PER power domain transition failing
+*
+* The clock framework should take care of ensuring
+* that the clock domain is in the right state when
+* GFX driver is active.
Are you suggesting that the clock framework is not doing this already?
No. This clkdm_*() calls are here to work-around an issue that I observed
when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls
clock domain across every suspend-resume is something I don't think can
be pushed to the clock framework.
+*/
+ clkdm_wakeup(gfx_l4ls_clkdm);
+ clkdm_sleep(gfx_l4ls_clkdm);
+
+ if (ret) {
+ pr_err(Kernel suspend failure\n);
+ } else {
+ status = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
We're trying to git rid of direct control module register access, and
consolidate them into control.c (for an eventual move to a driver.) I
see you've mostly done that in other parts of the series, but here's one
that needs to move.
Yes, I somehow missed this one. Will take care of it in the next version.
+ status = IPC_RESP_MASK;
+ status = __ffs(IPC_RESP_MASK);
+
+ switch (status) {
+ case 0:
+ pr_info(Successfully put all powerdomains to target
state\n);
+ /*
+* XXX: Leads to loss of logic state in PER power domain
+* Use SOC specific ops for this?
+*/
huh?
Ah... this is more of a TODO. There's no previous state entered information
in the PRCM registers. So to ensure that the drivers get the right information
when they check with the PM layer about the loss of context and hence the need
to restore the registers, I need to update the logic and membank state counters
for the PER power domain manually. I was thinking of leveraging the SoC specific
power domain ops for doing this.
[...]
+
+ /* Give some time to M3 to respond. 500msec is a random value here */
random? really?
Sort of. I don't have any numbers from the h/w guys on the worst
case delay in getting an interrupt from M3 to MPU. At the same time
I want to handle the scenario where something goes wrong on the M3
side and it
RE: [PATCH v3 9/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Hi Paul,
On Fri, Feb 08, 2013 at 19:47:04, Paul Walmsley wrote:
Hi Vaibhav,
a comment on this one:
On Tue, 29 Jan 2013, Vaibhav Bedia wrote:
Add minimal APIs for writing to the IPC and the M3_TXEV registers
in the Control module. These will be used in a subsequent patch which
adds suspend-resume support for AM33XX.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Acked-by: Peter Korsgaard jac...@sunsite.dk
---
v3: Add Peter's Acked-by
v2: No change
arch/arm/mach-omap2/control.c | 20
arch/arm/mach-omap2/control.h | 41
+
2 files changed, 61 insertions(+)
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 2adb268..c5d54ae 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -604,3 +604,23 @@ int omap3_ctrl_save_padconf(void)
}
#endif /* CONFIG_ARCH_OMAP3 CONFIG_PM */
+
+#if defined(CONFIG_SOC_AM33XX) defined(CONFIG_PM)
+void am33xx_txev_eoi(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ACK, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+void am33xx_txev_enable(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ENABLE, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+void am33xx_wkup_m3_ipc_cmd(struct am33xx_ipc_data *data)
+{
+ omap_ctrl_writel(data-resume_addr, AM33XX_CONTROL_IPC_MSG_REG0);
+ omap_ctrl_writel(data-sleep_mode, AM33XX_CONTROL_IPC_MSG_REG1);
+ omap_ctrl_writel(data-param1, AM33XX_CONTROL_IPC_MSG_REG2);
+ omap_ctrl_writel(data-param2, AM33XX_CONTROL_IPC_MSG_REG3);
+}
Could you please add some kerneldoc-style comments to these functions so
others can understand what they're intended to do, any side-effects they
have, any prerequisites, etc.?
Sure. Will do that in the next version and post it separately.
Thanks,
Vaibhav
--
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: OMAP baseline test results for v3.8-rc4
On Thu, Feb 07, 2013 at 03:30:56, Paul Walmsley wrote: Hi Vaibhav, On Thu, 24 Jan 2013, Bedia, Vaibhav wrote: I could not track down U-Boot that you were using It's posted now at: http://www.pwsan.com/omap/bootloaders/beaglebone/u-boot/2011.09-9-gcf6e04d__20120803171543/ Care to try it? Thanks. Unfortunately, I'll be able to do this early next week only. However, using your build configs for v3.7 and v3.8-rcX I got the same observations i.e. v3.7 boots but others don't. One difference that I found was that post v3.7 the configs that you are using have CONFIG_EARLY_PRINTK set. Once I disabled that I was able to bootup v3.8-rc1/4 (didn't try rc2/3 but I suspect early_printk was the culprit there too). I checked with Santosh on this and he mentioned that for DT-only boot, which AM335x is, that's expected behavior. Can you update your AM335x-only config to disable CONFIG_EARLY_PRINTK Setting CONFIG_EARLY_PRINTK=n doesn't fix the problem I'm seeing. I also tried building a v3.8-rc6 kernel with the old v3.7-rc config that was used before; no luck. Ah, I was really hoping you wouldn't say that ;) or just skip earlyprintk option in the bootargs for now? Haven't tried this one yet. If you still have issues booting can you update your U-Boot to v2013.01 since things seem to be working fine at this point. Let's try to identify and get rid of bootloader dependencies in the kernel. They indicate that the kernel isn't initializing something appropriately, which could cause strange problems later. Agreed. It could also be that the boot-loader is doing something crazy but we do need to know what's the dependency. Regards, Vaibhav -- 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/5] ARM: OMAP2+: Display correct system timer name
Hi Jon, On Wed, Jan 30, 2013 at 22:34:27, Hunter, Jon wrote: Currently on boot, when displaying the name of the gptimer used for clockevents and clocksource timers, the timer ID is shown. However, when booting with device-tree, the timer ID is not used to select a gptimer but a timer property. Hence, it is possible that the timer selected when booting with device-tree does not match the ID shown. Therefore, instead display the HWMOD name of the gptimer and use the HWMOD name as the name of clockevent and clocksource timer (if a gptimer is used). Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/timer.c | 44 +-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 72c2ca1..18cb856 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -71,6 +71,9 @@ #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET0x14 #define NUMERATOR_DENUMERATOR_MASK 0xf000 +/* Timer name needs to be big enough to store a string of timerXX */ +static char timer_name[10]; + Why not move this inside omap_dm_timer_init_one()? Regards, Vaibhav -- 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 0/5] ARM: OMAP2+: System timer updates
Hi Jon, On Wed, Jan 30, 2013 at 22:34:26, Hunter, Jon wrote: This series consists mainly of clean-ups for clockevents and clocksource timers on OMAP2+ devices. The most significant change in functionality comes from the 5th patch which is changing the selection of the clocksource timer for OMAP3 and AM335x devices when gptimers are used for clocksource. This change came about from Vaibhav Bedia's series for AM335x [1]. See patch for more details on the exact nature of the change. Boot tested with and without device-tree on OMAP2420 H4, OMAP3430 SDP, OMAP3430 Beagle Board, OMAP4430 SDP and AM335x EVM (AM335x only supports device-tree boot). Thanks for working on this. I have couple of minor comments but with this series in place I can drop the patch for interchanging the timers for AM33xx and the suspend-resume handlers for the clockevent also don't need the sprintf() that I had :) Reviewed-and-Tested-by: Vaibhav Bedia vaibhav.be...@ti.com -- 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 5/5] ARM: OMAP3: Update clocksource timer selection
Hi Jon, On Wed, Jan 30, 2013 at 22:34:31, Hunter, Jon wrote: When booting with device-tree for OMAP3 and AM335x devices and a gptimer is used as the clocksource (which is always the case for AM335x), a gptimer located in a power domain that is not always-on is selected. Ideally we should use a gptimer located in a power domain that is always on (such as the wake-up domain) so that time can be maintained during a kernel suspend without keeping on additional power domains unnecessarily. In order to fix this so that we can select a gptimer located in a power domain that is always-on, the following changes were made ... 1. Currently, only when selecting a gptimer to use for a clockevent timer, do we pass a timer property that can be used to select a specific gptimer. Change this so that we can pass a property when selecting a gptimer to use for a clocksource timer too. 2. Currently, when selecting either a gptimer to use for a clockevent timer or a clocksource timer and no timer property is passed, then the first available timer is selected regardless of the properties it has. Change this so that if no properties are passed, then a timer that does not have additional features (such as always-on, dsp-irq, pwm, and secure) is selected. Please note that using a gptimer for both clocksource and clockevents can have a system power impact during idle. The reason being is that OMAP and AMxxx devices typically only have one gptimer in a power domain that is always-on. Therefore when the kernel is idle both the clocksource and clockevent timers will be active and this will keep additional power domains on. During kernel suspend, only the clocksource timer is active and therefore, it is better to use a gptimer in a power domain that is always-on for clocksource. It's always a pleasure reading the changelog in your patches :) [...] #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX) -OMAP_SYS_GP_TIMER_INIT(3, 1, timer_sys_ck, ti,timer-alwon, -2, timer_sys_ck); +OMAP_SYS_GP_TIMER_INIT(3, 2, timer_sys_ck, NULL, +1, timer_sys_ck, ti,timer-alwon); #endif Minor point... was the intention of changing of clkev_nr and clksrc_nr to make it consistent with what happens on AM33xx which is DT-only? Regards, Vaibhav -- 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/5] ARM: OMAP2+: Display correct system timer name
On Fri, Feb 01, 2013 at 14:23:43, Hunter, Jon wrote: [...] +/* Timer name needs to be big enough to store a string of timerXX */ +static char timer_name[10]; + Why not move this inside omap_dm_timer_init_one()? In the non-DT case, the name member of the clocksource/event struct will point to this array and so it needs to reside in memory permanently and not just temporary. Once we migrate completely to DT then we will be able to remove this completely. See following snippet ... - sprintf(name, timer%d, gptimer_id); - oh_name = name; + sprintf(timer_name, timer%d, gptimer_id); + *name = timer_name; Ok. But in case of non-DT boot if someone selects gptimers for both clkevt and clksrc, both the name members will end up pointing to the same memory location. To be specific, in the current code the clkevt timer name will point to the clksrc name. This won't be noticeable during boot since the clkevt name gets printed before it is over-written. Regards, Vaibhav -- 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 5/5] ARM: OMAP3: Update clocksource timer selection
On Fri, Feb 01, 2013 at 14:55:31, Hunter, Jon wrote: [...] I don't see this as being DT specific. It is more of a policy change to ensure a wake-up domain timer is used for clocksource when we are using gptimers for both clocksource and clockevents. It was your patch for AM335x that made me see the need for this, if that makes sense. May be I should have referenced that in the changelog. Sorry to be clear, I don't see the policy change as DT specific. In answer to your question, yes clkev_nr and clksrc_nr are changed so the policy it is consistent regardless of whether you use DT or not. In other words, an OMAP3 board using a gptimer for clocksource (such as cm-t3517) and does not use DT, would work the same as AM335x with DT. Ok. Thanks for clarifying. Regards, Vaibhav -- 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 v2 12/18] ARM: OMAP2+: timer: Add suspend-resume callbacks for clockevent device
On Wed, Jan 30, 2013 at 23:16:34, Hunter, Jon wrote: Ok fair enough. By the way, I posted a patch today [1] that will use the hwmod name as the clockevent timer name. Care to try on top of that patch and then we can eliminate the sprintf. Thanks. Will try it out later today. Regards, Vaibhav -- 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 v2 13/18] ARM: OMAP2+: AM33XX: timer: Interchance clkevt and clksrc timers
On Wed, Jan 30, 2013 at 23:19:34, Hunter, Jon wrote: By the way, this need to be applied on top of the fix I sent yesterday to pass the property. Ok. Thanks for pointing this out. -- 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] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Regards, Vaibhav -- 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] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Thanks this works perfectly: original-bone: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E bone-black: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E I just re-read the mail-chain and I am confused here. So the patch in question is meant for Bone-A4 which has PG1.0? Regards, Vaibhav -- 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] cpsw: Fix interrupt storm among other things
Hi Antoniou, On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Thanks this works perfectly: original-bone: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E bone-black: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E I just re-read the mail-chain and I am confused here. So the patch in question is meant for Bone-A4 which has PG1.0? It is a general bug fix. The problem was discovered only on the bone black which has PG2.0 silicon. The driver has been tested and it works on the original bone with PG1.0 as well. But Mugunthan mentioned that he doesn't see this on an EVM with PG2.0 silicon... is there any board dependency here? Regards, Vaibhav -- 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 v2 8/9] ARM: DTS: AM33XX: Add nodes for OCMC RAM and WKUP-M3
On Tue, Jan 29, 2013 at 13:50:44, Peter Korsgaard wrote:
Vaibhav == Vaibhav Bedia vaibhav.be...@ti.com writes:
Vaibhav Since AM33XX supports only DT-boot, this is needed
Vaibhav for the appropriate device nodes to be created.
Vaibhav Note: OCMC RAM is part of the PER power domain and supports
Vaibhav retention. The assembly code for low power entry/exit will
Vaibhav run from OCMC RAM. To ensure that the OMAP PM code does not
Vaibhav attempt to disable the clock to OCMC RAM as part of the
Vaibhav suspend process add the no_idle_on_suspend flag.
Vaibhav Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Vaibhav Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Vaibhav ---
Vaibhav v2: Add reg property
Vaibhav arch/arm/boot/dts/am33xx.dtsi | 14 +
Vaibhav 1 file changed, 14 insertions(+)
Vaibhav diff --git a/arch/arm/boot/dts/am33xx.dtsi
b/arch/arm/boot/dts/am33xx.dtsi
Vaibhav index c2f14e8..423f898 100644
Vaibhav --- a/arch/arm/boot/dts/am33xx.dtsi
Vaibhav +++ b/arch/arm/boot/dts/am33xx.dtsi
Vaibhav @@ -385,5 +385,19 @@
Vaibhav mac-address = [ 00 00 00 00 00 00 ];
Vaibhav };
Vaibhav };
Vaibhav +
Vaibhav + ocmcram: ocmcram@4030 {
Vaibhav + compatible = ti,ocmcram;
Vaibhav + reg = 0x4030 0x1;
Vaibhav + ti,hwmods = ocmcram;
Vaibhav + ti,no_idle_on_suspend;
Vaibhav + };
Vaibhav +
Vaibhav + wkup_m3: wkup_m3@44d0 {
Vaibhav + compatible = ti,wkup_m3;
Both of these compatible properties should probably use less generic
names, like:
ti,am3352-ocmcram
ti,am3352-wkup-m3 ('-' instead of '_')
Ok. Will do.
Thanks,
Vaibhav
--
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 v3 0/9] ARM: OMAP2+: AM33XX: Misc fixes/updates
On Tue, Jan 29, 2013 at 16:57:04, Shilimkar, Santosh wrote: Vaibhav, On Tuesday 29 January 2013 04:44 PM, Vaibhav Bedia wrote: Hi, The following patches were earlier posted as part the AM33XX suspend-resume support series [1]. Based on the suggestion from Santosh Shilimkar santosh.shilim...@ti.com i have split out the changes which update the various data files related to AM33XX support. These patches apply on top of v3.8-rc5 v1-v2: Address the comments received from Sergei Shtylyov and Peter Korsgaard. v2-v3: Address an additional comment from Peter Korgaard and add his Acked-by to the patches Just a suggestion here... I saw v2 of $subject series just today morning and in just few hours, I see v3 as well. You should at least let v2 settle down for a while :-) Collecting tags and minor update can always be done once you gather sufficient comments to warrant a re-spin. OK. v1 has been in the wild for more than 10 days without any more comments so I went aggressive on v3 :) Regards, Vaibhav -- 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 selection of clockevent timer when using device-tree
On Wed, Jan 30, 2013 at 01:53:11, Hunter, Jon wrote: Commit 9725f44 (ARM: OMAP: Add DT support for timer driver) added device-tree support for selecting a clockevent timer by property. However, the code is currently ignoring the property passed and selecting the first available timer found. Hence, for the OMAP3 beagle board timer-12 is not being selected as expected. Fix this problem by ensuring the timer property is passed to omap_get_timer_dt(). I thought that was intentional ;) and had this change in the clkevt-clksrc interchange patch for AM33xx. Anyways, this change works for me so... Tested-by: Vaibhav Bedia vaibhav.be...@ti.com -- 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: OMAP baseline test results for v3.8-rc4
On Fri, Jan 25, 2013 at 22:29:43, Tony Lindgren wrote: * Bedia, Vaibhav vaibhav.be...@ti.com [130123 06:35]: Hi Tony, On Tue, Jan 22, 2013 at 23:53:32, Tony Lindgren wrote: [...] But I should get *something* from the kernel before it starts trying to access the rootfs ? Here's something Kevin fixed but did not send it out before going to a vacation. Can you give it a try with earlyprintk enabled? Note that this does not help with no output early on, that sounds like a bug configuring the DEBUG_LL port somewhere. I think earlyprintk on AM335x has not been functional for a while [1]. Will the latest patches from you to enable multiplatform debug-ll be sufficient to test this out? I am trying to track down the boot issues reported and just want to be sure of the dependencies. Yes you should check with current linux next and select the DEBUG_LL port manually. Verified on linux-next that selecting the right UART port gives a functional early_console. (One of the rare cases where forcing a kernel panic early in the boot process makes sense ;) Regards, Vaibhav -- 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: OMAP baseline test results for v3.8-rc4
Hi Paul, On Tue, Jan 22, 2013 at 07:54:44, Paul Walmsley wrote: Hi guys, Regarding the AM33xx test failures with appended DTBs, it would be very helpful if especially the TI people could try reproducing the problem. Otherwise it's going to cause problems with merging any new AM33xx patches, since I won't be able to test them without additional work. Plus, this is something that used to work up until d01e4afd, so something isn't right. You'll need to use the bootloader that TI originally shipped with the BeagleBones: U-Boot 2011.09-9-gcf6e04d (Mar 08 2012 - 17:15:43) This is because many folks don't replace their bootloader. I do plan to add a test with a recent version of u-boot, but the kernel should not be dependent on the bootloader in any way to work correctly. If it is, then we need to document what u-boot version the kernel started to work with. The Kconfig that I use is here: http://www.pwsan.com/omap/testlogs/test_v3.8-rc4/20130120122039/build/am33xx_only/am33xx_only It's possible that there's something wrong with the Kconfig. It's basically just omap2plus_defconfig, but with all OMAP support for non-AM33xx turned off, and with the appended DTB and ATAG compatibility options turned on. Let's try to do this ASAP. That way, if it's some bootloader dependency or bug in the kernel, some fix can be merged during the v3.8-rc series, which is rapidly drawing to a close. I could not track down U-Boot that you were using so I used the v2013.01 tag for U-Boot. However, using your build configs for v3.7 and v3.8-rcX I got the same observations i.e. v3.7 boots but others don't. One difference that I found was that post v3.7 the configs that you are using have CONFIG_EARLY_PRINTK set. Once I disabled that I was able to bootup v3.8-rc1/4 (didn't try rc2/3 but I suspect early_printk was the culprit there too). I checked with Santosh on this and he mentioned that for DT-only boot, which AM335x is, that's expected behavior. Can you update your AM335x-only config to disable CONFIG_EARLY_PRINTK or just skip earlyprintk option in the bootargs for now? If you still have issues booting can you update your U-Boot to v2013.01 since things seem to be working fine at this point. Regards, Vaibhav -- 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: OMAP baseline test results for v3.8-rc4
Hi Tony, On Tue, Jan 22, 2013 at 23:53:32, Tony Lindgren wrote: [...] But I should get *something* from the kernel before it starts trying to access the rootfs ? Here's something Kevin fixed but did not send it out before going to a vacation. Can you give it a try with earlyprintk enabled? Note that this does not help with no output early on, that sounds like a bug configuring the DEBUG_LL port somewhere. I think earlyprintk on AM335x has not been functional for a while [1]. Will the latest patches from you to enable multiplatform debug-ll be sufficient to test this out? I am trying to track down the boot issues reported and just want to be sure of the dependencies. [1] http://marc.info/?l=linux-omapm=134642491119235w=2 [2] https://patchwork.kernel.org/patch/1896851/ Regards, Vaibhav -- 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: OMAP baseline test results for v3.8-rc4
Hi, On Tue, Jan 22, 2013 at 14:25:13, Peter Korsgaard wrote: Paul == Paul Walmsley p...@pwsan.com writes: Paul Hi guys, Paul Regarding the AM33xx test failures with appended DTBs, it would Paul be very helpful if especially the TI people could try reproducing Paul the problem. Otherwise it's going to cause problems with merging Paul any new AM33xx patches, since I won't be able to test them Paul without additional work. Plus, this is something that used to Paul work up until d01e4afd, so something isn't right. Paul You'll need to use the bootloader that TI originally shipped with Paul the BeagleBones: Paul U-Boot 2011.09-9-gcf6e04d (Mar 08 2012 - 17:15:43) FYI, my beaglebone came with a slightly different U-Boot: U-Boot 2011.09-0-gf63b270-dirty (Nov 14 2011 - 10:37:14) But I have the same behaviour. Recent kernels work with a modern U-Boot, but not the original. The build I'm doing is very similar to yours: git describe v3.8-rc4-71-g9a92841 make ARCH=arm CROSS_COMPILE=arm-linux- omap2plus_defconfig echo CONFIG_ARM_APPENDED_DTB=y .config echo CONFIG_ARM_ATAG_DTB_COMPAT=y .config yes ''| make ARCH=arm CROSS_COMPILE=arm-linux- oldconfig make ARCH=arm CROSS_COMPILE=arm-linux- cat arch/arm/boot/dts/am335x-bone.dtb arch/arm/boot/zImage make ARCH=arm CROSS_COMPILE=arm-linux- uImage # old u-boot (ethernet not stable here, so load from sd) U-Boot SPL 2011.09-0-gf63b270-dirty (Nov 14 2011 - 10:37:14) Texas Instruments Revision detection unimplemented No AC power, disabling frequency switch OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img U-Boot 2011.09-0-gf63b270-dirty (Nov 14 2011 - 10:37:14) I2C: ready DRAM: 256 MiB No daughter card present NAND: HW ECC Hamming Code selected nand_get_flash_type: unknown NAND device: Manufacturer ID: 0x10, Chip ID: 0x10 No NAND device found!!! 0 MiB MMC: OMAP SD/MMC: 0 *** Warning - readenv() failed, using default environment Net: cpsw Hit any key to stop autoboot: 0 U-Boot# mmc rescan U-Boot# fatload mmc 0:1 0x8020 uImage.new reading uImage.new 3945127 bytes read U-Boot# setenv bootargs console=$console U-Boot# bootm 0x8020 ## Booting kernel from Legacy Image at 8020 ... Image Name: Linux-3.8.0-rc4-00071-g9a92841 Image Type: ARM Linux Kernel Image (uncompressed) Data Size:3945063 Bytes = 3.8 MiB Load Address: 80008000 Entry Point: 80008000 Verifying Checksum ... OK Loading Kernel Image ... OK OK Starting kernel ... And it hangs. With a reasonably modern U-Boot it works: I just re-built U-Boot from f63b270 and the kernel from 9a92841 using commands similar to Peter's and the kernel boots for me with the appended DTB. (For some reason U-Boot version string doesn't have the commit id and I can't recollect what causes this) U-Boot SPL 2011.09 (Jan 22 2013 - 18:06:56) Texas Instruments Revision detection unimplemented OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img U-Boot 2011.09 (Jan 22 2013 - 16:00:25) I2C: ready DRAM: 256 MiB WARNING: Caches not enabled No daughter card present NAND: HW ECC Hamming Code selected nand_get_flash_type: unknown NAND device: Manufacturer ID: 0x10, Chip ID: 0x10 No NAND device found!!! 0 MiB MMC: OMAP SD/MMC: 0 *** Warning - readenv() failed, using default environment Net: cpsw Hit any key to stop autoboot: 0 U-Boot# U-Boot# U-Boot# setenv bootargs console=$console U-Boot# setenv serverip 172.24.133.119 U-Boot# setenv bootfile uImage U-Boot# dhcp 8020 link up on port 0, speed 100, full duplex BOOTP broadcast 1 DHCP client bound to address 172.24.190.59 Using cpsw device TFTP from server 172.24.133.119; our IP address is 172.24.190.59; sending through gateway 172.24.188.1 Filename 'uImage'. Load address: 0x8020 Loading: # # # # # # # # # # # ### done Bytes transferred = 3917327 (3bc60f hex) U-Boot# bootm 0x8020 ## Booting kernel from Legacy Image at 8020 ... Image Name: Linux-3.8.0-rc4-00071-g9a92841 Image Type: ARM Linux Kernel Image (uncompressed) Data Size:3917263 Bytes = 3.7 MiB
RE: OMAP baseline test results for v3.8-rc4
On Tue, Jan 22, 2013 at 15:45:08, Mark Jackson wrote: On 22/01/13 02:24, Paul Walmsley wrote: Hi guys, Regarding the AM33xx test failures with appended DTBs, it would be very helpful if especially the TI people could try reproducing the problem. My non-working setup (I'm using a recent U-Boot) is as follows ... [Note that the DTB patch mentioned elsewhere in this thread seems to be *already* applied to -next] $ git describe next-20130121 $ make -j 8 ARCH=arm CROSS_COMPILE=arm-linux- omap2plus_defconfig $ make -j 8 ARCH=arm CROSS_COMPILE=arm-linux- menuconfig CONFIG_ARM_APPENDED_DTB=y CONFIG_ARM_ATAG_DTB_COMPAT=y CONFIG_EARLY_PRINTK=y $ make -j 8 ARCH=arm CROSS_COMPILE=arm-linux- $ cat arch/arm/boot/zImage arch/arm/boot/dtb/am335x-bone.dtb arch/arm/boot/zImage-dtb.am335x-bone $ scripts/mkuboot.sh -A arm -O linux -C none -T kernel -a 0×80008000 -e 0×80008000 -n ‘Linux’ -d arch/arm/boot/zImage-dtb.am335x-bone arch/arm/boot/uImage-dtb.am335x-bone $ cp arch/arm/boot/uImage-dtb.am335x-bone /tftpboot/nanobone/uImage-dtb And when I boot:- U-Boot SPL 2013.01 (Jan 16 2013 - 15:20:58) OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img U-Boot 2013.01 (Jan 16 2013 - 15:20:58) I2C: ready DRAM: 256 MiB WARNING: Caches not enabled NAND: No NAND device found!!! 0 MiB MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 *** Warning - readenv() failed, using default environment musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn) musb-hdrc: MHDRC RTL version 2.0 musb-hdrc: setup fifo_mode 4 musb-hdrc: 28/31 max ep, 16384/16384 memory USB Peripheral mode controller at 47401000 using PIO, IRQ 0 musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn) musb-hdrc: MHDRC RTL version 2.0 musb-hdrc: setup fifo_mode 4 musb-hdrc: 28/31 max ep, 16384/16384 memory USB Host mode controller at 47401800 using PIO, IRQ 0 Net: cpsw, usb_ether Hit any key to stop autoboot: 0 mmc0 is current device SD/MMC found on device 0 reading uEnv.txt 167 bytes read in 3 ms (53.7 KiB/s) Loaded environment from uEnv.txt Importing environment from mmc ... Running uenvcmd ... cpsw Waiting for PHY auto negotiation to complete. done link up on port 0, speed 100, full duplex BOOTP broadcast 1 BOOTP broadcast 2 *** Unhandled DHCP Option in OFFER/ACK: 44 *** Unhandled DHCP Option in OFFER/ACK: 46 *** Unhandled DHCP Option in OFFER/ACK: 44 *** Unhandled DHCP Option in OFFER/ACK: 46 DHCP client bound to address 10.0.0.112 Using cpsw device TFTP from server 10.0.0.100; our IP address is 10.0.0.112 Filename '/nanobone/uImage-dtb'. Load address: 0x8000 Loading: # # # # ### 627.9 KiB/s done Bytes transferred = 3963919 (3c7c0f hex) ## Booting kernel from Legacy Image at 8000 ... Image Name: Linux Image Type: ARM Linux Kernel Image (uncompressed) Data Size:3963855 Bytes = 3.8 MiB Load Address: 80008000 Entry Point: 80008000 Verifying Checksum ... OK Loading Kernel Image ... OK OK Starting kernel ... Following works for me: Kernel === git checkout next-20130122 make distclean make omap2plus_defconfig Enable the appended DTB related options via menuconfig make -j7 cat arch/arm/boot/zImage arch/arm/boot/dts/am335x-bone.dtb arch/arm/boot/zImage-dtb.am335x-bone mkimage -A arm -O linux -C none -T kernel -a 0x80008000 -e 0x80008000 -n 'Linux' -d arch/arm/boot/zImage-dtb.am335x-bone arch/arm/boot/uImage-dtb.am335x-bone U-Boot === Built from v2013.01 Bootlog === U-Boot SPL 2013.01 (Jan 22 2013 - 18:47:57) OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img U-Boot 2013.01 (Jan 22 2013 - 18:47:57) I2C: ready DRAM: 256 MiB WARNING: Caches not enabled NAND: No NAND device found!!! 0 MiB MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 *** Warning - readenv() failed, using default environment musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn) musb-hdrc: MHDRC RTL version 2.0 musb-hdrc: setup fifo_mode 4 musb-hdrc: 28/31 max ep, 16384/16384 memory USB Peripheral mode controller at 47401000 using PIO, IRQ 0 musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn) musb-hdrc: MHDRC RTL version 2.0 musb-hdrc: setup fifo_mode 4 musb-hdrc: 28/31 max ep, 16384/16384 memory USB Host mode controller at 47401800 using PIO, IRQ 0 Net: cpsw, usb_ether Hit any key to stop autoboot: 0 U-Boot# setenv serverip 172.24.133.119 U-Boot# setenv bootfile uImage-dtb.am335x-bone U-Boot# dhcp 8020 link up on port 0, speed 100, full duplex
RE: [PATCH 0/9] ARM: OMAP2+: AM33XX: Misc fixes/updates
Hi Peter, On Mon, Jan 21, 2013 at 14:33:20, Peter Korsgaard wrote: V == Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi, Vaibhav Bedia (9): ARM: OMAP2+: AM33XX: CM: Get rid of unncessary header inclusions ARM: OMAP2+: AM33XX: CM/PRM: Use __ASSEMBLER__ macros in header files ARM: OMAP2+: AM33XX: hwmod: Register OCMC RAM hwmod ARM: OMAP2+: AM33XX: hwmod: Update TPTC0 hwmod with the right flags ARM: OMAP2+: AM33XX: hwmod: Fixup cpgmac0 hwmod entry ARM: OMAP2+: AM33XX: hwmod: Update the WKUP-M3 hwmod with reset status bit ARM: OMAP2+: AM33XX: Update the hardreset API ARM: DTS: AM33XX: Add nodes for OCMC RAM and WKUP-M3 ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs V Any comments on these before I resend addressing the comments from Sergei? Wouldn't the ocmram/wkup dt nodes need a reg property to define their size as I indirectly mentioned here: http://thread.gmane.org/gmane.linux.ports.arm.omap/91405/focus=92061 My apologies. I missed your comment on that patch and just responded to it. Will add the reg property for the DT nodes in the next version. Regards, Vaibhav -- 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 v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Peter,
On Thu, Jan 17, 2013 at 19:57:20, Peter Korsgaard wrote:
V == Vaibhav Bedia vaibhav.be...@ti.com writes:
Hi,
V +static void am33xx_pm_firmware_cb(const struct firmware *fw, void
*context)
V +{
V + struct wkup_m3_context *wkup_m3_context = context;
V + struct platform_device *pdev = to_platform_device(wkup_m3_context-dev);
V + int ret = 0;
V +
V + /* no firmware found */
V + if (!fw) {
V + dev_err(wkup_m3_context-dev, request_firmware failed\n);
V + goto err;
V + }
V +
V + memcpy((void *)wkup_m3_context-code, fw-data, fw-size);
A size check would be good. I don't know much about the finer details
about the m3 and how it is connected, but don't you need to ensure data
is flushed before resetting the m3?
Will add the reg property in the next version. The wkup-m3 memory is coming via
ioremap() so AFAIK it should be ok. I realized that I do need to replace the
memcpy()
with memcpy_toio() here.
Thanks,
Vaibhav
--
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 0/9] ARM: OMAP2+: AM33XX: Misc fixes/updates
Hi Paul, Benoit, On Fri, Jan 18, 2013 at 12:49:20, Bedia, Vaibhav wrote: Hi, The following patches were earlier posted as part the AM33XX suspend-resume support series [1]. Based on the suggestion from Santosh Shilimkar santosh.shilim...@ti.com i have split out the changes which update the various data files related to AM33XX support. These patches apply on top of v3.8-rc3 Regards, Vaibhav [1] http://marc.info/?l=linux-arm-kernelm=135698501821074w=2 Vaibhav Bedia (9): ARM: OMAP2+: AM33XX: CM: Get rid of unncessary header inclusions ARM: OMAP2+: AM33XX: CM/PRM: Use __ASSEMBLER__ macros in header files ARM: OMAP2+: AM33XX: hwmod: Register OCMC RAM hwmod ARM: OMAP2+: AM33XX: hwmod: Update TPTC0 hwmod with the right flags ARM: OMAP2+: AM33XX: hwmod: Fixup cpgmac0 hwmod entry ARM: OMAP2+: AM33XX: hwmod: Update the WKUP-M3 hwmod with reset status bit ARM: OMAP2+: AM33XX: Update the hardreset API ARM: DTS: AM33XX: Add nodes for OCMC RAM and WKUP-M3 ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs Any comments on these before I resend addressing the comments from Sergei? Regards, Vaibhav -- 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/9] ARM: OMAP2+: AM33XX: hwmod: Update TPTC0 hwmod with the right flags
On Fri, Jan 18, 2013 at 17:58:49, Sergei Shtylyov wrote:
Hello.
On 18-01-2013 11:19, Vaibhav Bedia wrote:
Third Party Transfer Controller (TPTC0) needs to be idled and
put to standby under SW control. Add the appropriate flags in
the TPTC0 hwmod entry.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
---
Change from RFC version:
Clarify TPTC in the changelog
arch/arm/mach-omap2/omap_hwmod_33xx_data.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
index 8280f11..f2f408c 100644
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -1823,6 +1823,7 @@ static struct omap_hwmod am33xx_tptc0_hwmod = {
.class = am33xx_tptc_hwmod_class,
.clkdm_name = l3_clkdm,
.mpu_irqs = am33xx_tptc0_irqs,
+ .flags = (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY),
Parens not needed here.
Will drop it in v2.
Regards,
Vaibhav
--
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/9] ARM: OMAP2+: AM33XX: CM: Get rid of unncessary header inclusions
On Fri, Jan 18, 2013 at 18:01:01, Sergei Shtylyov wrote: On 18-01-2013 11:19, Vaibhav Bedia wrote: Some of the included header files are not needed so remove them. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Acked-by: Santosh Shilimkar santosh.shilim...@ti.com --- Change from RFC version: No change arch/arm/mach-omap2/cm33xx.h |7 +-- 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-omap2/cm33xx.h b/arch/arm/mach-omap2/cm33xx.h index 5fa0b62..8009e13 100644 --- a/arch/arm/mach-omap2/cm33xx.h +++ b/arch/arm/mach-omap2/cm33xx.h @@ -17,16 +17,11 @@ #ifndef __ARCH_ARM_MACH_OMAP2_CM_33XX_H #define __ARCH_ARM_MACH_OMAP2_CM_33XX_H -#include linux/delay.h -#include linux/errno.h -#include linux/err.h -#include linux/io.h - #include common.h #include cm.h #include cm-regbits-33xx.h -#include cm33xx.h +#include iomap.h You don't comment on this addition in the changelog... Hmm... ok. Will clarify this in the changelog for v2. Regards, Vaibhav -- 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 v2 12/18] ARM: OMAP2+: timer: Add suspend-resume callbacks for clockevent device
Hi Jon,
On Fri, Jan 18, 2013 at 00:10:40, Hunter, Jon wrote:
On 12/31/2012 07:07 AM, Vaibhav Bedia wrote:
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Vaibhav Hiremath hvaib...@ti.com
Cc: Jon Hunter jon-hun...@ti.com
---
v1-v2:
Get rid of harcoded timer id.
Note: since a platform device is not created for these timer
instances and because there's very minimal change needed for
restarting the timer a full blown context save and restore
has been skipped.
arch/arm/mach-omap2/timer.c | 33 +
1 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 691aa67..38f9cbc 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -128,6 +128,36 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, clkev.id);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ __omap_dm_timer_stop(clkev, 1, clkev.rate);
I am not sure you need to call __omap_dm_timer_stop() here. This should
have already been called as timekeeping_suspend() will call
omap2_gp_timer_set_mode() to shutdown the timer.
You are right, i can drop the call to __omap_dm_timer_stop().
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, clkev.id);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
Similarly here, I am not sure these __omap_dm_timer_ calls are needed.
I went through the code again. __omap_dm_timer_load_start() is invoked
from omap2_gp_timer_set_mode() with the auto-reload flag when setting the
mode to CLOCK_EVT_MODE_PERIODIC and without the auto-reload flag in
omap2_gp_timer_set_next_event(). So looks like this call can be dropped.
But I do need the call to __omap_dm_timer_int_enable() to re-enable the
interrupts from the timer.
+}
+
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -135,6 +165,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
AFAIK, this is only applicable for AM335x devices and so should not be
added for all.
Agreed. Will address this in the next version.
};
static struct property device_disabled = {
@@ -323,6 +355,7 @@ static void __init omap2_gp_clockevent_init(int
gptimer_id,
int res;
clkev.errata = omap_dm_timer_get_errata();
+ clkev.id = gptimer_id;
We should not use gptimer_id anymore. This will go away once the
migration to dev-tree is completed. You may be better off storing the
oh_name in the clock_event_device name field and passing to the
suspend/resume handlers.
Currently the name field in clock_event_device is set to gp_timer. Should I
set
the name in omap2_gp_clockevent_init() based on the gptimer_id?
Regards,
Vaibhav
--
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 v2 12/18] ARM: OMAP2+: timer: Add suspend-resume callbacks for clockevent device
On Fri, Jan 18, 2013 at 10:55:43, Shilimkar, Santosh wrote:
On Friday 18 January 2013 12:15 AM, Jon Hunter wrote:
On 01/10/2013 10:37 PM, Bedia, Vaibhav wrote:
On Tue, Jan 08, 2013 at 20:45:10, Shilimkar, Santosh wrote:
On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote:
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Vaibhav Hiremath hvaib...@ti.com
Cc: Jon Hunter jon-hun...@ti.com
---
v1-v2:
Get rid of harcoded timer id.
Note: since a platform device is not created for these timer
instances and because there's very minimal change needed for
restarting the timer a full blown context save and restore
has been skipped.
arch/arm/mach-omap2/timer.c | 33 +
1 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 691aa67..38f9cbc 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -128,6 +128,36 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+char name[10];
+struct omap_hwmod *oh;
+
+sprintf(name, timer%d, clkev.id);
+oh = omap_hwmod_lookup(name);
+if (!oh)
+return;
+
+__omap_dm_timer_stop(clkev, 1, clkev.rate);
+omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+char name[10];
+struct omap_hwmod *oh;
+
+sprintf(name, timer%d, clkev.id);
+oh = omap_hwmod_lookup(name);
+if (!oh)
+return;
+
+omap_hwmod_enable(oh);
+__omap_dm_timer_load_start(clkev,
+OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+__omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
Am still bit uncomfortable with direct hwmod usage in the suspend/resmue
hooks.
Jon, Any alternatives you can think of ?
Jon,
Any suggestions here?
Sorry completed this missed this!
Unfortunately, I don't have any good suggestions here. However, I am
wondering if the suspend/resume handlers can just be calls to
omap_hwmod_idle/enable and we can remove these __omap_dm_timer_
calls (I don't think they are needed). Furthermore, my understanding is
this is only needed for AM335x (per the changelog), and so we should not
add suspend/resume handlers for all OMAP2+ based devices.
I agree with the direction.
I need to retain the call to enable the interrupt but the others can be dropped.
Will take care of this and the comment on invoking the suspend/resume handlers
only for AM335x in the next version.
Regards,
Vaibhav
--
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 v2 12/18] ARM: OMAP2+: timer: Add suspend-resume callbacks for clockevent device
On Tue, Jan 08, 2013 at 20:45:10, Shilimkar, Santosh wrote:
On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote:
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Vaibhav Hiremath hvaib...@ti.com
Cc: Jon Hunter jon-hun...@ti.com
---
v1-v2:
Get rid of harcoded timer id.
Note: since a platform device is not created for these timer
instances and because there's very minimal change needed for
restarting the timer a full blown context save and restore
has been skipped.
arch/arm/mach-omap2/timer.c | 33 +
1 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 691aa67..38f9cbc 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -128,6 +128,36 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, clkev.id);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ __omap_dm_timer_stop(clkev, 1, clkev.rate);
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, clkev.id);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
Am still bit uncomfortable with direct hwmod usage in the suspend/resmue
hooks.
Jon, Any alternatives you can think of ?
Jon,
Any suggestions here?
Regards,
Vaibhav
--
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 v2 14/18] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Wed, Jan 09, 2013 at 13:01:03, Shilimkar, Santosh wrote: On Wednesday 09 January 2013 11:08 AM, Bedia, Vaibhav wrote: On Tue, Jan 08, 2013 at 20:51:08, Shilimkar, Santosh wrote: On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote: Add minimal APIs for writing to the IPC and the M3_TXEV registers in the Control module. These will be used in a subsequent patch which adds suspend-resume support for AM33XX. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Tony Lingren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: Vaibhav Hiremath hvaib...@ti.com --- On Control module, we are trying to move driver/module specific code to respective drivers. Can you not add below code to ipc related driver component. If not, then patch as such is fine with me. I had it in the pm33xx.c in the previous version. Kevin had asked me to move it to control.c. Should I revert move it back there? pm33xx.c is not the right place. I was hoping to move to some driver. If that is not possible then leave it in control module. I think I'll leave it here for now. Could I have you ack in that case? Thanks, Vaibhav -- 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/9] mailbox: OMAP: introduce mailbox framework
Hi Loic, On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Sure, I'll. When do you plan to post an updated version of these patches? Regards, Vaibhav -- 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/9] mailbox: OMAP: introduce mailbox framework
On Wed, Jan 09, 2013 at 17:59:39, Loic PALLARDY wrote: Hi Vaibhav, On 01/09/2013 01:11 PM, Bedia, Vaibhav wrote: Hi Loic, On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Sure, I'll. When do you plan to post an updated version of these patches? I'm synchronizing with Omar to include TI RPMsg and tidspbridge patches in next update. So I plan update for end of the week, beginning of next week. Ok. Thanks. Regards, Vaibhav -- 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 v2 01/18] mailbox: OMAP2+: Add support for AM33XX
On Tue, Jan 08, 2013 at 19:23:44, Shilimkar, Santosh wrote:
On Monday 31 December 2012 06:36 PM, Vaibhav Bedia wrote:
Mailbox IP on AM33XX is the same as that present in OMAP4.
The single instance of Mailbox IP on AM33XX contains
8 sub-modules and facilitates communication between MPU,
PRUs and WKUP_M3.
PRUS?
Programmable Real Time Units. Will clarify in next version
The first mailbox sub-module is assigned for communication
between MPU and WKUP-M3.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Russ Dill russ.d...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
---
v1-v2:
Address the comment on operator usage from Russ Dill
drivers/mailbox/mailbox-omap2.c | 35 ++-
1 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/drivers/mailbox/mailbox-omap2.c
b/drivers/mailbox/mailbox-omap2.c
index 7c26bed..6d61159 100644
--- a/drivers/mailbox/mailbox-omap2.c
+++ b/drivers/mailbox/mailbox-omap2.c
@@ -151,7 +151,7 @@ static void omap2_mbox_disable_irq(struct mailbox *mbox,
struct omap_mbox2_priv *p = mbox-priv;
u32 bit = (irq == IRQ_TX) ? p-notfull_bit : p-newmsg_bit;
- if (!cpu_is_omap44xx())
+ if (!cpu_is_omap44xx() !soc_is_am33xx())
bit = mbox_read_reg(p-irqdisable) ~bit;
mbox_write_reg(bit, p-irqdisable);
@@ -352,6 +352,32 @@ struct mailbox mbox_2_info = {
struct mailbox *omap4_mboxes[] = { mbox_1_info, mbox_2_info, NULL };
#endif
+#if defined(CONFIG_SOC_AM33XX)
+static struct omap_mbox2_priv omap2_mbox_wkup_m3_priv = {
+ .tx_fifo = {
+ .msg= MAILBOX_MESSAGE(0),
+ .fifo_stat = MAILBOX_FIFOSTATUS(0),
+ .msg_stat = MAILBOX_MSGSTATUS(0),
+ },
+ .rx_fifo = {
+ .msg= MAILBOX_MESSAGE(1),
+ .msg_stat = MAILBOX_MSGSTATUS(1),
+ },
+ .irqenable = OMAP4_MAILBOX_IRQENABLE(3),
+ .irqstatus = OMAP4_MAILBOX_IRQSTATUS(3),
+ .irqdisable = OMAP4_MAILBOX_IRQENABLE_CLR(3),
+ .notfull_bit= MAILBOX_IRQ_NOTFULL(0),
+ .newmsg_bit = MAILBOX_IRQ_NEWMSG(0),
+};
+
+struct mailbox mbox_wkup_m3_info = {
+ .name = wkup_m3,
+ .ops= omap2_mbox_ops,
+ .priv = omap2_mbox_wkup_m3_priv,
+};
+struct mailbox *am33xx_mboxes[] = { mbox_wkup_m3_info, NULL };
+#endif
+
static int __devinit omap2_mbox_probe(struct platform_device *pdev)
{
struct resource *mem;
@@ -386,6 +412,13 @@ static int __devinit omap2_mbox_probe(struct
platform_device *pdev)
list[0]-irq = list[1]-irq = platform_get_irq(pdev, 0);
}
#endif
+#if defined(CONFIG_SOC_AM33XX)
ifdef in middle of the code. I know you are just
following what is already there.
+ else if (soc_is_am33xx()) {
+ list = am33xx_mboxes;
+
UN-necessary extra line here.
Will remove
+ list[0]-irq = platform_get_irq(pdev, 0);
+ }
+#endif
Hopefully mailbox clean-up will kill that #ifdeffery.
Apart from those comments, patch looks fine to me.
Thanks,
Vaibhav
--
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 v2 02/18] mailbox: Add an API for flushing the FIFO
On Tue, Jan 08, 2013 at 19:26:51, Shilimkar, Santosh wrote:
On Monday 31 December 2012 06:36 PM, Vaibhav Bedia wrote:
On AM33XX, the mailbox module between the MPU and the
WKUP-M3 co-processor facilitates a one-way communication.
MPU uses the assigned mailbox sub-module to issue the
interrupt to the WKUP-M3 co-processor which then goes
and reads the the IPC data from registers in the control
module.
WKUP-M3 is in the L4_WKUP and does not have any access to
the mailbox module. Due to this limitation, the MPU is
completely responsible for FIFO maintenance and interrupt
generation. MPU needs to ensure that the FIFO does not
overflow by reading back the assigned mailbox sub-module.
This patch adds an API in the mailbox code which the MPU
can use to empty the FIFO by issuing a readback command.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
---
Note: This patch which will be slightly reworked once the mailbox
driver changes are finalized.
Can you expand a bit please ?
There could be some changes in the structure names.
drivers/mailbox/mailbox-omap2.c | 19 +++
drivers/mailbox/mailbox.c | 36
drivers/mailbox/mailbox.h |3 +++
include/linux/mailbox.h |1 +
4 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/drivers/mailbox/mailbox-omap2.c
b/drivers/mailbox/mailbox-omap2.c
index 6d61159..c732be1 100644
--- a/drivers/mailbox/mailbox-omap2.c
+++ b/drivers/mailbox/mailbox-omap2.c
@@ -125,6 +125,23 @@ static int omap2_mbox_fifo_full(struct mailbox *mbox)
return mbox_read_reg(fifo-fifo_stat);
}
+static int omap2_mbox_fifo_needs_flush(struct mailbox *mbox)
+{
+ struct omap_mbox2_priv *p = mbox-priv;
+ struct omap_mbox2_fifo *fifo = p-tx_fifo;
+
+ return mbox_read_reg(fifo-msg_stat);
+}
+
+static void omap2_mbox_fifo_readback(struct mailbox *mbox,
+ struct mailbox_msg *msg)
+{
+ struct omap_mbox2_priv *p = mbox-priv;
+ struct omap_mbox2_fifo *fifo = p-tx_fifo;
+
+ msg-header = mbox_read_reg(fifo-msg);
+}
+
static int ompa2_mbox_poll_for_space(struct mailbox *mbox)
{
if (omap2_mbox_fifo_full(mbox))
@@ -221,6 +238,8 @@ static struct mailbox_ops omap2_mbox_ops = {
.read = omap2_mbox_fifo_read,
.write = omap2_mbox_fifo_write,
.empty = omap2_mbox_fifo_empty,
+ .fifo_needs_flush = omap2_mbox_fifo_needs_flush,
+ .fifo_readback = omap2_mbox_fifo_readback,
.poll_for_space = ompa2_mbox_poll_for_space,
.enable_irq = omap2_mbox_enable_irq,
.disable_irq= omap2_mbox_disable_irq,
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 2f50226..92c9f68 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -57,6 +57,15 @@ static inline int mbox_empty(struct mailbox *mbox)
{
return mbox-ops-empty(mbox);
}
+static inline int mbox_fifo_needs_flush(struct mailbox *mbox)
+{
+ return mbox-ops-fifo_needs_flush(mbox);
+}
+static inline void mbox_fifo_readback(struct mailbox *mbox,
+ struct mailbox_msg *msg)
+{
+ mbox-ops-fifo_readback(mbox, msg);
+}
/* Mailbox IRQ handle functions */
static inline void ack_mbox_irq(struct mailbox *mbox, mailbox_irq_t irq)
@@ -110,6 +119,33 @@ out:
}
EXPORT_SYMBOL(mailbox_msg_send);
+/*
s/*/**
Will do.
+ * Flush the Rx FIFO by reading back the messages
+ * Since the normal expectation is that the Rx will do the
+ * reading, add a debug message to indicate if we really flush
+ *
+ * Returns the no. of messages read back
+ */
Just look at the kernel doc style for above
Rest looks fine.
Ok.
Thanks,
Vaibhav
--
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 v2 03/18] memory: emif: Move EMIF related header file to include/linux/
On Tue, Jan 08, 2013 at 19:34:41, Shilimkar, Santosh wrote:
[...]
drivers/memory/emif.c |2 +-
drivers/memory/emif.h | 589
---
include/linux/ti_emif.h | 589
+++
You are just moving the file. So git mv file1 flie2; and the git
format-patch -C
on committed patch should just generate few lines of patch.
Ok. Didn't know about this.
+/* DDR_PHY_CTRL_1 - EMIF4D5 */
+#define DLL_HALF_DELAY_SHIFT_4D5 21
+#define DLL_HALF_DELAY_MASK_4D5(1 21)
+#define READ_LATENCY_SHIFT_4D5 0
+#define READ_LATENCY_MASK_4D5 (0x1f 0)
+
+/* DDR_PHY_CTRL_1_SHDW */
+#define DDR_PHY_CTRL_1_SHDW_SHIFT 5
+#define DDR_PHY_CTRL_1_SHDW_MASK (0x7ff 5)
+#define READ_LATENCY_SHDW_SHIFT0
+#define READ_LATENCY_SHDW_MASK (0x1f 0)
+
+#ifndef __ASSEMBLY__
+/*
+ * Structure containing shadow of important registers in EMIF
+ * The calculation function fills in this structure to be later used for
+ * initialisation and DVFS
+ */
+struct emif_regs {
Are you using above struct. If not we can leave it in same place and
just move the register defines.
No, the struct is not used. I'll leave it here in the next version.
Regards,
Vaibhav
--
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 v2 05/18] ARM: OMAP2+: AM33XX: CM/PRM: Use __ASSEMBLER__ macros in header files
On Tue, Jan 08, 2013 at 20:31:44, Shilimkar, Santosh wrote: [...] +#endif /* ASSEMBLER */ + Drop that extra line. Ok. #endif diff --git a/arch/arm/mach-omap2/prm33xx.h b/arch/arm/mach-omap2/prm33xx.h index 3f25c56..2f2eaa0 100644 --- a/arch/arm/mach-omap2/prm33xx.h +++ b/arch/arm/mach-omap2/prm33xx.h @@ -117,6 +117,7 @@ #define AM33XX_PM_CEFUSE_PWRSTST_OFFSET 0x0004 #define AM33XX_PM_CEFUSE_PWRSTST AM33XX_PRM_REGADDR(AM33XX_PRM_CEFUSE_MOD, 0x0004) +#ifndef __ASSEMBLER__ extern u32 am33xx_prm_read_reg(s16 inst, u16 idx); extern void am33xx_prm_write_reg(u32 val, s16 inst, u16 idx); extern u32 am33xx_prm_rmw_reg_bits(u32 mask, u32 bits, s16 inst, s16 idx); @@ -126,4 +127,6 @@ extern int am33xx_prm_is_hardreset_asserted(u8 shift, s16 inst, extern int am33xx_prm_assert_hardreset(u8 shift, s16 inst, u16 rstctrl_offs); extern int am33xx_prm_deassert_hardreset(u8 shift, s16 inst, u16 rstctrl_offs, u16 rstst_offs); +#endif /* ASSEMBLER */ + ditto Ok. Otherwise looks fine. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Thanks, Vaibhav -- 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 v2 13/18] ARM: OMAP2+: AM33XX: timer: Interchance clkevt and clksrc timers
On Tue, Jan 08, 2013 at 20:47:28, Shilimkar, Santosh wrote: On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote: AM33XX has two timers (DTIMER0/1) in the WKUP domain. On GP devices the source of DMTIMER0 is fixed to an inaccurate internal 32k RC oscillator and this makes the DMTIMER0 practically either as a clocksource or as clockevent. Currently the timer instance in WKUP domain is used as the clockevent and the timer in non-WKUP domain as the clocksource. DMTIMER1 in WKUP domain can keep running in suspend from a 32K clock fed from external OSC and can serve as the persistent clock for the kernel. To enable this, interchange the timers used as clocksource and clockevent for AM33XX. For now a new DT property has been added to allow the timer code to select the timer with the right property. It has been pointed out by Santosh Shilimkar and Kevin Hilman that such a change will result in soc-idle never being achieved on AM33XX. There are other reasons why soc-idle does not look feasible on AM33XX so for now we go ahead with the interchange of the the timers. If at a later point of time we do come up with an approach which makes soc-idle possible on AM33XX, this can be revisited. Can you please explain other reasons as well for clarity ? I guess from h/w perspective it boils down to lack of HW-AUTO and IO-Daisy chaining on AM33xx [1]. Since there's no 32ksynctimer, do we also need to register DMTIMER1 as the persistent clock on AM33xx? This can only be done from DMTIMER1 which is currently serving as the clockevent. Regards, Vaibhav [1] http://marc.info/?l=linux-arm-kernelm=135238055717206w=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
RE: [RFC v2 00/18] ARM: OMAP2+: AM33XX: Add suspend-resume support
Hi Santosh, On Tue, Jan 08, 2013 at 21:01:51, Shilimkar, Santosh wrote: Vaibhav, On Monday 31 December 2012 06:36 PM, Vaibhav Bedia wrote: Hi, This is the second version of the patch series for adding suspend-resume support for AM33XX. Based on the feedback received on the previous patch series [1] almost all the patches have undergone a bit a rework. The 1st two patches depend on the changes for mailbox code migration from arch/arm/*-omap*/ to drivers/mailbox/ [2]. The patch series also depends on recent changes to the OMAP PM framework by Paul Walmsley. I found it easiest to apply the AM33XX suspend-resume patches on top of Paul's TEST_pwrdm_post_fpwrst_devel_a_3.9 branch + the patches @ [2]. With these dependencies met, the PM code uses the firmware interface and expects the userspace to load the WKUP_M3 binary before the suspend-resume functionality is made available. The binary file (and the source-code for WKUP_M3) can be obtained from the 'next' branch at [3]. The WKUP_M3 binary can either be loaded post bootup via the sysfs entry './sys/devices/ocp.2/wkup_m3.4/firmware' or it can be included in the kernel image as part of the build process. DDR3 specific changes have been skipped for now since mainline U-Boot exhibited stability issues on all the DDR3 based AM335x boards that i could lay my hands on. I have done basic testing along with power measurments on the different power rails on the AM335x EVM. PER domain transition on the BeagleBone fails if the CPSW driver is included in the kernel and is yet to be root caused. Along with this issue more extensive testing on other OMAP platforms is also pending right now. For more details on the AM335x suspend-resume support please refer to the changelog in the different patches. I still haven't reviewed patch 15, 16 and 18 which adds suspend support. Will do that in coming days since they need a bit a closer look. But as mentioned in some of the patches, you need to split this series since except 15, 16 and 18 which adds suspend support, rest of the patches are - data file fixes - timer suspend/resume update - mailbox support, control module update. Would be good to split the series to help the reviews. Sure. I'll split it up like you mentioned. Since all these changes are needed for a working suspend-resume I kept it in a single series to reduce dependencies and also to help set an initial context for the AM33xx PM changes. Regards, Vaibhav -- 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 v2 17/18] ARM: OMAP2+: AM33XX: Select Mailbox when PM is enabled
On Tue, Jan 08, 2013 at 20:52:37, Shilimkar, Santosh wrote: On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote: PM services on AM33XX depend on mailbox for communication with WKUP-M3 core so ensure that the right config options are selected. Thanks to Kevin Hilman khil...@deeprootsystems.com for the suggestion on updating the Kconfig and not just the omap2plus_defconfig which was done in the previous version of the AM33XX suspend-resume support. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Tony Lingren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com --- Unrelated to series. You can post this patch separately. Ok. I kept it in this series so that it does not look to be some random patch. Regards, Vaibhav -- 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 v2 14/18] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tue, Jan 08, 2013 at 20:51:08, Shilimkar, Santosh wrote: On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote: Add minimal APIs for writing to the IPC and the M3_TXEV registers in the Control module. These will be used in a subsequent patch which adds suspend-resume support for AM33XX. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Tony Lingren t...@atomide.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: Vaibhav Hiremath hvaib...@ti.com --- On Control module, we are trying to move driver/module specific code to respective drivers. Can you not add below code to ipc related driver component. If not, then patch as such is fine with me. I had it in the pm33xx.c in the previous version. Kevin had asked me to move it to control.c. Should I revert move it back there? Regards, Vaibhav -- 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 v2 07/18] ARM: OMAP2+: AM33XX: hwmod: Update TPTC0 hwmod with the right flags
On Tue, Jan 08, 2013 at 20:35:39, Shilimkar, Santosh wrote: On Monday 31 December 2012 06:37 PM, Vaibhav Bedia wrote: TPTC0 needs to be idled and put to standby under SW control. Add the appropriate flags in the TPTC0 hwmod entry. Can you please expand TPTC0 in chane log. Third Party Transfer Controller. It's part of the DMA IP. Will add it in the changelog. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: Vaibhav Hiremath hvaib...@ti.com --- Patch is fine otherwise. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- 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 v2 01/18] mailbox: OMAP2+: Add support for AM33XX
Hi Tony, On Tue, Jan 01, 2013 at 23:55:06, Tony Lindgren wrote: [...] mbox_write_reg(bit, p-irqdisable); The cpu_is/soc_is macros are no longer available to drivers with 8d91a42e (Merge tag 'omap-late-cleanups'...). So you'll have to pass whatever flags the driver needs in platform_data or as device tree properties. Thanks for the heads-up. Will rework this patch once the mailbox driver migration is finalized. Regards, Vaibhav -- 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 02/12] ARM: OMAP2+: PM: introduce power domains functional states
Hi Paul, A minor comment below. On Sun, Dec 09, 2012 at 23:23:01, Paul Walmsley wrote: [...] + + pr_debug(powerdomain: convert pwrst (%0x,%0x) to fpwrst %0x\n, + pwrst, logic, *fpwrst); + This function alone does not print the powerdomain name. Can you add that in the final version? Regards, Vaibhav -- 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 09/12] ARM: OMAP2+: powerdomain: skip register reads for powerdomains known to be on
Hi Paul,
On Mon, Dec 10, 2012 at 01:33:28, Paul Walmsley wrote:
There's no need to determine the current power state for powerdomains
that must be on while the kernel is running. We mark these
powerdomains with a new flag, PWRDM_ACTIVE_WITH_KERNEL. Any
powerdomain marked with that flag is reported as being in the ON power
state while the kernel is running.
[...]
diff --git a/arch/arm/mach-omap2/powerdomains33xx_data.c
b/arch/arm/mach-omap2/powerdomains33xx_data.c
index 869adb8..acb148a 100644
--- a/arch/arm/mach-omap2/powerdomains33xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains33xx_data.c
@@ -123,7 +123,8 @@ static struct powerdomain mpu_33xx_pwrdm = {
.pwrstst_offs = AM33XX_PM_MPU_PWRSTST_OFFSET,
.pwrsts = PWRSTS_OFF_RET_ON,
.pwrsts_logic_ret = PWRSTS_OFF_RET,
- .flags = PWRDM_HAS_LOWPOWERSTATECHANGE,
+ .flags = (PWRDM_HAS_LOWPOWERSTATECHANGE |
+PWRDM_ACTIVE_WITH_KERNEL),
In case of AM33xx, this flag should be set for PER and WKUP pwrdms also.
EMIF, L3, L4 etc come under PER so this domain can't transition on an active
system. PRCM and Control module come under WKUP, so the WKUP should the kernel
should attempt to transition WKUP domain also.
Regards,
Vaibhav
RE: [PATCH 09/12] ARM: OMAP2+: powerdomain: skip register reads for powerdomains known to be on
On Wed, Dec 26, 2012 at 11:51:46, Bedia, Vaibhav wrote:
Hi Paul,
On Mon, Dec 10, 2012 at 01:33:28, Paul Walmsley wrote:
There's no need to determine the current power state for powerdomains
that must be on while the kernel is running. We mark these
powerdomains with a new flag, PWRDM_ACTIVE_WITH_KERNEL. Any
powerdomain marked with that flag is reported as being in the ON power
state while the kernel is running.
[...]
diff --git a/arch/arm/mach-omap2/powerdomains33xx_data.c
b/arch/arm/mach-omap2/powerdomains33xx_data.c
index 869adb8..acb148a 100644
--- a/arch/arm/mach-omap2/powerdomains33xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains33xx_data.c
@@ -123,7 +123,8 @@ static struct powerdomain mpu_33xx_pwrdm = {
.pwrstst_offs = AM33XX_PM_MPU_PWRSTST_OFFSET,
.pwrsts = PWRSTS_OFF_RET_ON,
.pwrsts_logic_ret = PWRSTS_OFF_RET,
- .flags = PWRDM_HAS_LOWPOWERSTATECHANGE,
+ .flags = (PWRDM_HAS_LOWPOWERSTATECHANGE |
+ PWRDM_ACTIVE_WITH_KERNEL),
In case of AM33xx, this flag should be set for PER and WKUP pwrdms also.
EMIF, L3, L4 etc come under PER so this domain can't transition on an active
system. PRCM and Control module come under WKUP, so
... the kernel should not attempt to change the WKUP power domain state.
Regards,
Vaibhav
--
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 09/10] ARM: OMAP2+: clockdomain: convert existing atomic usecounts into spinlock-protected shorts/ints
Hi Paul,
On Sun, Dec 09, 2012 at 06:53:43, Paul Walmsley wrote:
The atomic usecounts seem to be confusing, and are no longer needed
since the operations that they are attached to really should take
place under lock. Replace the atomic counters with simple integers,
protected by the enclosing powerdomain spinlock.
[...]
@@ -1063,7 +1123,8 @@ static int _clkdm_clk_hwmod_enable(struct clockdomain
*clkdm)
* should be called for every clock instance or hwmod that is
* enabled, so the clkdm can be force woken up.
*/
- if ((atomic_inc_return(clkdm-usecount) 1) autodeps) {
+ clkdm-usecount++;
+ if (clkdm-usecount 1 autodeps) {
pwrdm_unlock(clkdm-pwrdm.ptr);
return 0;
}
This is not directly related to this patch but something I noticed when I
enabled
the various debug prints.
if the clkdm-usecount is 1, there is still a call to
arch_clkdm-clkdm_clk_enable().
Won't the usecount 1 guarantee that the clkdm is in the right state and the
PRCM
access can be skipped?
Regards,
Vaibhav
--
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 5/9] mailbox: change protection mechanisms
On Tue, Dec 18, 2012 at 18:40:08, Loic Pallardy wrote: TX: replace spin by mutex RX: replace spin_lock_irq by spin_lock_irqsave Can you please add a short note on why this is being done? Regards, Vaibhav -- 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/9] mailbox: create opened message type
On Tue, Dec 18, 2012 at 18:40:07, Loic Pallardy wrote:
Current message type is a u32 to fit HW fifo format.
This should be extended to support any message exchanges
and type of mailbox.
Propose structure owns the original u32 and an optional
pointer on additional data.
Signed-off-by: Loic Pallardy loic.palla...@st.com
---
drivers/mailbox/Kconfig | 9 ++
drivers/mailbox/mailbox-omap1.c | 18 ++--
drivers/mailbox/mailbox-omap2.c | 10 ---
drivers/mailbox/mailbox.c | 64
+
drivers/mailbox/mailbox.h | 6 ++--
include/linux/mailbox.h | 17 ++-
6 files changed, 89 insertions(+), 35 deletions(-)
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index d1e7d74..efb766f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -33,4 +33,13 @@ config MBOX_KFIFO_SIZE
This can also be changed at runtime (via the mbox_kfifo_size
module parameter).
+config MBOX_DATA_SIZE
+ int Mailbox associated data max size (bytes)
+ default 64
+ help
+ Specify the default size of mailbox's associated data buffer
+ (bytes)
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
endif
diff --git a/drivers/mailbox/mailbox-omap1.c b/drivers/mailbox/mailbox-omap1.c
index 31cb70a..94e90af 100644
--- a/drivers/mailbox/mailbox-omap1.c
+++ b/drivers/mailbox/mailbox-omap1.c
@@ -50,26 +50,26 @@ static inline void mbox_write_reg(u32 val, size_t ofs)
}
/* msg */
-static mbox_msg_t omap1_mbox_fifo_read(struct mailbox *mbox)
+static int omap1_mbox_fifo_read(struct mailbox *mbox, struct mailbox_msg
*msg)
{
struct omap_mbox1_fifo *fifo =
((struct omap_mbox1_priv *)mbox-priv)-rx_fifo;
- mbox_msg_t msg;
- msg = mbox_read_reg(fifo-data);
- msg |= ((mbox_msg_t) mbox_read_reg(fifo-cmd)) 16;
+ msg-header = mbox_read_reg(fifo-data);
+ msg-header |= ((mbox_msg_t) mbox_read_reg(fifo-cmd)) 16;
Now that struct mailbox_msg encapsulates the data, you can
get rid of the mbox_msg_t typedef completely. Having the data
as part of the mailbox_msg along with the functions with mbox_msg_t
as the return type just creates confusion IMHO.
- return msg;
+ return 0;
}
Convert all return 0 functions to void?
[...]
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
index 18c9502..d256e1a 100644
--- a/include/linux/mailbox.h
+++ b/include/linux/mailbox.h
@@ -7,7 +7,22 @@ typedef int __bitwise mailbox_irq_t;
#define IRQ_TX ((__force mailbox_irq_t) 1)
#define IRQ_RX ((__force mailbox_irq_t) 2)
-int mailbox_msg_send(struct mailbox *, mbox_msg_t msg);
+struct mailbox_msg {
+ int size;
+ u32 header;
+ void*pdata;
+};
+
+#define MAILBOX_FILL_MSG(_msg, _header, _pdata, _size) { \
+ _msg.header = _header; \
+ _msg.pdata = (void *)_pdata; \
+ _msg.size = _size; \
+}
+
+#define MAILBOX_FILL_HEADER_MSG(_msg, _header) \
+ MAILBOX_FILL_MSG(_msg, _header, NULL, 0);
+
I used these patches as part of the suspend-resume support for AM335x
which has the same mailbox IP as OMAP4. I used the MAILBOX_FILL_HEADER_MSG
helper and things work as expected.
However, I found the 'header' part to be very confusing. Why not treat the
OMAP case as a special case of the new MAILBOX_FILL_MSG where the data size
is set to 1?
Regards,
Vaibhav
--
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/9] mailbox: OMAP: introduce mailbox framework
On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: On 12/18/2012 05:59 PM, Tony Lindgren wrote: * Loic Pallardyloic.pallardy-...@stericsson.com [121218 05:15]: Signed-off-by: Omar Ramirez Lunaomar.l...@linaro.org AFAIK the first two patches should have: From: Omar Ramirez Lunaomar.l...@linaro.org Yes right, my mistake. I'll fix that on next version. /Loic I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Thanks, Vaibhav -- 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 compillation error in mach-omap2/timer.c
Hi Peter, Tony On Wed, Dec 19, 2012 at 15:20:09, Ujfalusi, Peter wrote: prom_add_property() has been renamed to of_add_property() This patch fixes the following comilation error: arch/arm/mach-omap2/timer.c: In function ‘omap_get_timer_dt’: arch/arm/mach-omap2/timer.c:178:3: error: implicit declaration of function ‘prom_add_property’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[1]: *** [arch/arm/mach-omap2/timer.o] Error 1 make[1]: *** Waiting for unfinished jobs Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com --- Hi Tony, today's mainline does not compile due to this API rename in of core. Not sure if we alreasy have fix for this pending. There following patches for needed to get omap2plus_defconfig build working https://patchwork.kernel.org/patch/1810481/ - address the build breakage due to header file cleanup https://patchwork.kernel.org/patch/1810441/ - address the prom_add_property() issue http://marc.info/?l=linux-arm-kernelm=135589966017628w=2 - this crept in during a merge commit.
BUG: spinlock bad magic on CPU#0 on BeagleBone
Hi, Current mainline on Beaglebone using the omap2plus_defconfig + 3 build fixes is triggering a BUG() [0.00] Booting Linux on physical CPU 0x0 [0.00] Linux version 3.7.0-01415-g55bc169 (a0393953@psplinux063) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #1 SMP Wed Dec 19 13:40:13 IST 2012 [0.00] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine: Generic AM33XX (Flattened Device Tree), model: TI AM335x BeagleBone [0.00] Memory policy: ECC disabled, Data cache writeback [0.00] On node 0 totalpages: 65280 [0.00] free_area_init_node: node 0, pgdat c07b3680, node_mem_map c0d11000 [0.00] Normal zone: 512 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 64768 pages, LIFO batch:15 [0.00] AM335X ES1.0 (neon ) [0.00] PERCPU: Embedded 9 pages/cpu @c0f1a000 s12992 r8192 d15680 u36864 [0.00] pcpu-alloc: s12992 r8192 d15680 u36864 alloc=9*4096 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 64768 [0.00] Kernel command line: console=ttyO0,115200n8 mem=256M root=/dev/ram rw initrd=0x8200,16MB ramdisk_size=65536 earlyprintk=serial [0.00] PID hash table entries: 1024 (order: 0, 4096 bytes) [0.00] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes) [0.00] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes) [0.00] __ex_table already sorted, skipping sort [0.00] Memory: 255MB = 255MB total [0.00] Memory: 229012k/229012k available, 33132k reserved, 0K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xd080 - 0xff00 ( 744 MB) [0.00] lowmem : 0xc000 - 0xd000 ( 256 MB) [0.00] pkmap : 0xbfe0 - 0xc000 ( 2 MB) [0.00] modules : 0xbf00 - 0xbfe0 ( 14 MB) [0.00] .text : 0xc0008000 - 0xc06d3f3c (6960 kB) [0.00] .init : 0xc06d4000 - 0xc07252c0 ( 325 kB) [0.00] .data : 0xc0726000 - 0xc07b6068 ( 577 kB) [0.00].bss : 0xc07b608c - 0xc0d10af0 (5483 kB) [0.00] Hierarchical RCU implementation. [0.00] RCU restricting CPUs from NR_CPUS=2 to nr_cpu_ids=1. [0.00] NR_IRQS:16 nr_irqs:16 16 [0.00] IRQ: Found an INTC at 0xfa20 (revision 5.0) with 128 interrupts [0.00] Total of 128 interrupts on 1 active controller [0.00] OMAP clockevent source: GPTIMER1 at 2400 Hz [0.00] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms [0.00] OMAP clocksource: GPTIMER2 at 2400 Hz [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.001227] Calibrating delay loop... 364.48 BogoMIPS (lpj=1425408) [0.109084] pid_max: default: 32768 minimum: 301 [0.109688] Security Framework initialized [0.109889] Mount-cache hash table entries: 512 [0.112674] BUG: spinlock bad magic on CPU#0, swapper/0/0 [0.112724] lock: atomic64_lock+0x240/0x400, .magic: , .owner: none/-1, .owner_cpu: 0 [0.112782] [c001af64] (unwind_backtrace+0x0/0xf0) from [c02c2010] (do_raw_spin_lock+0x158/0x198) [0.112813] [c02c2010] (do_raw_spin_lock+0x158/0x198) from [c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58) [0.112844] [c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58) from [c02cabf0] (atomic64_add_return+0x30/0x5c) [0.112886] [c02cabf0] (atomic64_add_return+0x30/0x5c) from [c0124564] (alloc_mnt_ns.clone.14+0x44/0xac) [0.112914] [c0124564] (alloc_mnt_ns.clone.14+0x44/0xac) from [c0124f4c] (create_mnt_ns+0xc/0x54) [0.112951] [c0124f4c] (create_mnt_ns+0xc/0x54) from [c06f31a4] (mnt_init+0x120/0x1d4) [0.112978] [c06f31a4] (mnt_init+0x120/0x1d4) from [c06f2d50] (vfs_caches_init+0xe0/0x10c) [0.113005] [c06f2d50] (vfs_caches_init+0xe0/0x10c) from [c06d4798] (start_kernel+0x29c/0x300) [0.113029] [c06d4798] (start_kernel+0x29c/0x300) from [80008078] (0x80008078) [0.118290] CPU: Testing write buffer coherency: ok [0.118968] CPU0: thread -1, cpu 0, socket -1, mpidr 0 [0.119053]
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 01:53:42, Stephen Boyd wrote:
On 12/19/12 08:53, Paul Walmsley wrote:
On Wed, 19 Dec 2012, Bedia, Vaibhav wrote:
Current mainline on Beaglebone using the omap2plus_defconfig + 3 build
fixes
is triggering a BUG()
...
[0.109688] Security Framework initialized
[0.109889] Mount-cache hash table entries: 512
[0.112674] BUG: spinlock bad magic on CPU#0, swapper/0/0
[0.112724] lock: atomic64_lock+0x240/0x400, .magic: , .owner:
none/-1, .owner_cpu: 0
[0.112782] [c001af64] (unwind_backtrace+0x0/0xf0) from [c02c2010]
(do_raw_spin_lock+0x158/0x198)
[0.112813] [c02c2010] (do_raw_spin_lock+0x158/0x198) from
[c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58)
[0.112844] [c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58) from
[c02cabf0] (atomic64_add_return+0x30/0x5c)
[0.112886] [c02cabf0] (atomic64_add_return+0x30/0x5c) from
[c0124564] (alloc_mnt_ns.clone.14+0x44/0xac)
[0.112914] [c0124564] (alloc_mnt_ns.clone.14+0x44/0xac) from
[c0124f4c] (create_mnt_ns+0xc/0x54)
[0.112951] [c0124f4c] (create_mnt_ns+0xc/0x54) from [c06f31a4]
(mnt_init+0x120/0x1d4)
[0.112978] [c06f31a4] (mnt_init+0x120/0x1d4) from [c06f2d50]
(vfs_caches_init+0xe0/0x10c)
[0.113005] [c06f2d50] (vfs_caches_init+0xe0/0x10c) from [c06d4798]
(start_kernel+0x29c/0x300)
[0.113029] [c06d4798] (start_kernel+0x29c/0x300) from [80008078]
(0x80008078)
[0.118290] CPU: Testing write buffer coherency: ok
[0.118968] CPU0: thread -1, cpu 0, socket -1, mpidr 0
[0.119053] Setting up static identity map for 0x804de2c8 - 0x804de338
[0.120698] Brought up 1 CPUs
This is probably a memory corruption bug, there's probably some code
executing early that's writing outside its own data and trashing some
previously-allocated memory.
I'm not so sure. It looks like atomic64s use spinlocks on processors
that don't have 64-bit atomic instructions (see lib/atomic64.c). And
those spinlocks are not initialized until a pure initcall runs,
init_atomic64_lock(). Pure initcalls don't run until after
vfs_caches_init() and so you get this BUG() warning that the spinlock is
not initialized.
How about we initialize the locks statically? Does that fix your problem?
8-
diff --git a/lib/atomic64.c b/lib/atomic64.c
index 9785378..08a4f06 100644
--- a/lib/atomic64.c
+++ b/lib/atomic64.c
@@ -31,7 +31,11 @@
static union {
raw_spinlock_t lock;
char pad[L1_CACHE_BYTES];
-} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp;
+} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
+ [0 ... (NR_LOCKS - 1)] = {
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
+ },
+};
static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
{
@@ -173,14 +177,3 @@ int atomic64_add_unless(atomic64_t *v, long long a, long
long u)
return ret;
}
EXPORT_SYMBOL(atomic64_add_unless);
-
-static int init_atomic64_lock(void)
-{
- int i;
-
- for (i = 0; i NR_LOCKS; ++i)
- raw_spin_lock_init(atomic64_lock[i].lock);
- return 0;
-}
-
-pure_initcall(init_atomic64_lock);
I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) have
DDR2
and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same point.
With Stephen's change I don't see this on any of the board variants :)
New bootlog below.
Thanks,
Vaibhav
---
[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 3.7.0-01415-g55bc169-dirty (a0393953@psplinux063)
(gcc version 4.5.3 20110311 (prerelease) (GCC) ) #4 SMP Thu Dec 20 09:59:12 IST
2012
[0.00] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[0.00] Machine: Generic AM33XX (Flattened Device Tree), model: TI
AM335x BeagleBone
[0.00] Memory policy: ECC disabled, Data cache writeback
[0.00] AM335X ES1.0 (neon )
[0.00] PERCPU: Embedded 9 pages/cpu @c0f1a000 s12992 r8192 d15680 u36864
[0.00] Built 1 zonelists in Zone order, mobility grouping on. Total
pages: 64768
[0.00] Kernel command line: console=ttyO0,115200n8 mem=256M
root=/dev/ram rw initrd=0x8200,16MB ramdisk_size=65536 earlyprintk=serial
[0.00] PID hash table entries: 1024 (order: 0, 4096 bytes)
[0.00] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[0.00] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[0.00] __ex_table already sorted, skipping sort
[0.00] Memory: 255MB = 255MB total
[0.00] Memory: 229012k/229012k available, 33132k reserved, 0K highmem
[0.00] Virtual kernel memory layout:
[0.00] vector : 0x - 0x1000 ( 4 kB)
[0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB)
[0.00
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 11:55:24, Stephen Boyd wrote: On 12/19/2012 8:48 PM, Bedia, Vaibhav wrote: I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) have DDR2 and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same point. With Stephen's change I don't see this on any of the board variants :) New bootlog below. Great! Can I have your Tested-by then? I'll wrap it up into a patch. Is this is a new regression? From a glance at the code it looks to have existed for quite a while now. I went back to a branch based off 3.7-rc4 and don't see the issue there. Not sure what is triggering this now. Tested-by: Vaibhav Bedia vaibhav.be...@ti.com -- 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/1] ARM: OMAP: Fix build breakage due to missing include in i2c.c
On Wed, Dec 19, 2012 at 12:14:58, Bedia, Vaibhav wrote: Merge commit 752451f (Merge branch 'i2c-embedded/for-next' of git://git.pengutronix.de/git/wsa/linux) resulted in a build breakage for OMAP ... arch/arm/mach-omap2/i2c.c: In function 'omap_pm_set_max_mpu_wakeup_lat_compat': arch/arm/mach-omap2/i2c.c:130:2: error: implicit declaration of function 'omap_pm_set_max_mpu_wakeup_lat' make[1]: *** [arch/arm/mach-omap2/i2c.o] Error 1 ... Fix this by including the appropriate header file with the function prototype Reported-by: Fengguang Wu fengguang...@intel.com Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com I messed up the LAKML address in this version. Resent with the right address. -- 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 v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
Hi Benoit, On Mon, Nov 26, 2012 at 14:32:59, Cousson, Benoit wrote: Hi Vaibhav, On 11/26/2012 06:19 AM, Bedia, Vaibhav wrote: On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: As part of PWM subsystem integration, PWM subsystem are sharing resources like clock across submodules (ECAP, EQEP EHRPWM). To handle resource sharing IP integration 1. Rework on parent child relation between PWMSS and ECAP, EQEP EHRPWM child devices to support runtime PM. 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional clock gating from control module. 3. Add HWMOD entries for EQEP PWM submodule. Is there any review on this patch? This patch depends on ECAP EHRPWM to work in am335x. First of all, I think you should break up this patch as per the 3 points that you mentioned above. The usage of opt_clks for this does not look right to me. Based on your description this clock is necessary and not optional on AM335x and on Davinci platforms this clock does not exist. I checked the DA830 TRM and looks like TBCLK for eHRPWM is an always on clock there. So, the only difference in AM335x is an additional enable bit. Instead of adding this as opt_clk in hwmod, we could add an always on clock node in Davinci clock data and have the driver always do a clk_enable() on the tbclk as part of the probe sequence. On AM335x, with the right clock node this will enable the clock in hardware and on DA830 it turns into a NOP. This way we can avoid adding the opt_clk entry in hwmod of eHRPWM. I think the custom activate/deactivate functions in the OMAP runtime PM implementation was a good fit for keeping this SoC integration detail out of the driver code. However, the current DT flow in omap_device.c seems to assign the default activate/deactivate ops. Is that approach deprecated? The issue is that this approach is not doable anymore with DT, that's why I had to provide a default set of functions. So once all OMAP drivers get converted to DT, will there be no notion of latency based activate/deactivate functions? Or will it get used in a different manner? Regards, Vaibhav -- 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 v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: As part of PWM subsystem integration, PWM subsystem are sharing resources like clock across submodules (ECAP, EQEP EHRPWM). To handle resource sharing IP integration 1. Rework on parent child relation between PWMSS and ECAP, EQEP EHRPWM child devices to support runtime PM. 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional clock gating from control module. 3. Add HWMOD entries for EQEP PWM submodule. Is there any review on this patch? This patch depends on ECAP EHRPWM to work in am335x. First of all, I think you should break up this patch as per the 3 points that you mentioned above. The usage of opt_clks for this does not look right to me. Based on your description this clock is necessary and not optional on AM335x and on Davinci platforms this clock does not exist. I think the custom activate/deactivate functions in the OMAP runtime PM implementation was a good fit for keeping this SoC integration detail out of the driver code. However, the current DT flow in omap_device.c seems to assign the default activate/deactivate ops. Is that approach deprecated? Regards, Vaibhav -- 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 v3 1/5] rtc: OMAP: Add system pm_power_off to rtc driver
On Thu, Nov 22, 2012 at 11:17:02, AnilKumar, Chimata wrote: +Andrew Morton On Tue, Nov 20, 2012 at 15:18:43, AnilKumar, Chimata wrote: From: Colin Foe-Parker colin.foepar...@logicpd.com Add system power off control to rtc driver which is the in-charge of controlling the BeagleBone system power. The power_off routine can be hooked up to pm_power_off system call. System power off sequence:- * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low * Enable PMIC_POWER_EN in rtc module * Set rtc ALARM2 time * Enable ALARM2 interrupt Added while (1); after the above steps to make sure that no other process acquire cpu. Otherwise we might see an unexpected behaviour because we are shutting down all the power rails of SoC except RTC. Reviewed-by: Vaibhav Bedia vaibhav.be...@ti.com -- 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 15/15] ARM: OMAP2+: AM33XX: Basic suspend resume support
On Wed, Nov 07, 2012 at 22:45:20, Kevin Hilman wrote: [...] We could perhaps add a couple of APIs to check the SYSC values when coming out of suspend and take appropriate action if the sysc cache does not match? Yes, for IPs with only SW support and no drivers, we may need something like this. But again, it needs to be suspend and idle aware, not just suspend. Ok, if the pwrmdm pre and post transition callbacks do this that should take care of both suspend and idle. Regards, Vaibhav -- 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 11/15] ARM: OMAP: timer: Interchange clksrc and clkevt for AM33XX
Hi Santosh, On Tue, Nov 06, 2012 at 20:05:40, Bedia, Vaibhav wrote: Hi Santosh, On Tue, Nov 06, 2012 at 03:29:22, Shilimkar, Santosh wrote: [...] IMO, assuming that idle will not be useful from the begining is leading down the path to poor design choices that will be much more difficult to fixup down the road in order to add idle support later. We need to design both idle and suspend at the same time. I agree with Kevin. Not supporting CPUIDLE deep states can hit the active power numbers dearly. I just don't know why the SOCs don't share the standard and must have design choices. But thats another discussion. Yes, active power numbers are not comparable to OMAP :( How about leaving the timer choices as is. PER timer for clock source and wakeuptimer for clock event. Anyway in suspend the clock-source can be suspended and that is evident from recent discussion. The only downside is you won't count time in suspend which is any way the case. Vaibhav, Do you guys see any implementation bottleneck for above ? Looking at the timekeeping code I see one more potential reason for making this change. OMAP registers the 32k sync timer as the persistent clock and since there's no 32k sync timer in AM33xx it doesn't register a persistent clock right now. Based on what I understood, we need to have to register one and DMTimer1 is the only clock that can serve as the persistent clock in suspend state. When we do so we might as well use it as the clocksource. A related question that I had was, is there a mechanism to handle the 32k counter (DMTimer or sync timer) wraparound condition in suspend? Does interchanging the clksrc and clkevt look fine to you? Regards, Vaibhav -- 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 15/15] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Kevin, On Wed, Nov 07, 2012 at 06:36:06, Kevin Hilman wrote: Bedia, Vaibhav vaibhav.be...@ti.com writes: On Mon, Nov 05, 2012 at 23:10:27, Kevin Hilman wrote: [...] Also, if there are drivers for these devices, won't this interfere? Hmm, I can think of the following scenarios 1. Runtime PM adapted drivers are compiled in - We'll have to ensure that in their suspend callbacks they end up doing omap_hwmod_idle() via the runtime PM APIs. That already happens for all omap_devices. In this case the omap_hwmod_enable() followed by omap_hwmod_idle() is not necessary in the PM code. Correct. 2. The drivers are not compiled in - In this case, the hwmod code puts the IPs in standby during bootup so the first suspend-resume iteration will pass. During resuming, the SYSC configuration for forced standby will be lost, Please clarify how the SYSC is lost in this case. The register configuration of IPs in the PER domain is lost when we enter the suspend state. So the forced standby configuration from SYSC is lost and we need to do this for every successful suspend-resume cycle. so in the subsequent iterations these IPs won't go standby and the hwmod code does not touch them since they never got enabled. In this case we do need the sequence that is there currently. 3. For some reason the respective drivers don't idle the IPs during suspend - (no_idle_on_suspend flag for the hwmod in DT?). In this scenario I think we should abort the suspend process since we know that the suspend is not going to work. Agreed. Is there some other scenario that needs to be covered? You covered the ones I was thinking of. How about adding some flag in hwmod code for handling this? Something similar to what was done for MMC [1]. I think the problem that we have is in some ways similar to the one addressed in [1]. Except that in the absence of drivers, no hwmod code is executed on the suspend/resume path. We could perhaps add a couple of APIs to check the SYSC values when coming out of suspend and take appropriate action if the sysc cache does not match? Regards, Vaibhav -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Jon, On Tue, Nov 06, 2012 at 02:50:50, Hunter, Jon wrote: [...] Why is this? How is the dmtimer TIOCP_CFG register configured on AM33xx? Is it using smart-idle? Yes, it is set to smart-idle with wakeup capable mode. (this needs a fixup since this timer is not wakeup capable) but unfortunately this is not sufficient. On AM33xx there's no HW_AUTO mode magic so unless the IPs in PER domain are disabled by s/w, PER domain can't transition. The next one is that the clockevent doesn't generate any further interrupts once the system resumes. We need to restore the pre-suspend configuration. I haven't tried but I guess we could have used the save and restore of timer registers here. It would be interesting to try using an non-wakeup domain timer on OMAP3/4 for clock events and seeing if suspend/resume works. Do you know what the exact problem here is? I understand that the timer context could get lost, but exactly what is not getting restarted by the kernel? For example, the only place we set the interrupt enable is during the clock event init and so if the context is lost, then I could see no more interrupts occurring. So is it enough to just restore the interrupt enable register, do you really need to program the timer again? Just restoring the interrupt enable register works. But since there's no logic retention I think a context save-restore would be better. Regards, Vaibhav -- 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/7] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
On Tue, Nov 06, 2012 at 13:42:21, N, Mugunthan V wrote:
[...]
+struct omap_hwmod_addr_space am33xx_mdio_addr_space[] = {
+ {
+ .pa_start = 0x4A101000,
+ .pa_end = 0x4A101000 + SZ_256 - 1,
+ .flags = ADDR_MAP_ON_INIT,
Based on the recent discussions and looking the hwmod code,
I guess ADDR_MAP_ON_INIT does not make sense here. Since you
are just creating a parent-child relationship here, maybe no
flag is needed?
+ },
+ { }
+};
+
+struct omap_hwmod_ocp_if am33xx_cpgmac0__mdio = {
+ .master = am33xx_cpgmac0_hwmod,
+ .slave = am33xx_mdio_hwmod,
+ .addr = am33xx_mdio_addr_space,
+ .user = OCP_USER_MPU,
Is this flag necessary? Shouldn't you just skip the
user field since there's nothing for the hwmod code
to do here?
Regards,
Vaibhav
--
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/8] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Mon, Nov 05, 2012 at 14:42:24, Philip, Avinash wrote:
[...]
+
+static struct omap_hwmod_ocp_if am33xx_epwmss0__ecap0 = {
+ .master = am33xx_epwmss0_hwmod,
+ .slave = am33xx_ecap0_hwmod,
+ .clk= l4ls_gclk,
+ .addr = am33xx_ecap0_addr_space,
+ .user = OCP_USER_MPU,
+};
Noticed this in another patch which is quite similar so commenting here
again. Is the user field required if you are just creating a parent-child
relationship here?
Regards,
Vaibhav
--
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 08/15] ARM: OMAP2+: hwmod: Fix the omap_hwmod_addr_space for CPGMAC0
On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote: [...] Ok I checked this one. The change I made was indirectly fixing another issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two entries and the SYSC register is part of the second entry. The function _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with the flag ADDR_TYPE_RT flag. The change I made indirectly made the second entry in am33xx_cpgmac0_addr_space[] become the first memory space with the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct SYSC address of CPGMAC0 and the IP went to standby during bootup. After changing the order of the entries in am33xx_cpgmac0_addr_space[] things work fine. Good catch. Just a side note on this, driver expects the addresses in this order only, first SS and then WR. Sorry I didn't understand your comment. For HWMOD code to work as expected, we need to change the order. Are you saying that I should not be doing this because the driver will get affected? Regards, Vaibhav -- 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 15/15] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Kevin,
On Mon, Nov 05, 2012 at 23:10:27, Kevin Hilman wrote:
[...]
First, some general comments. This is a big patch and probably should
be broken up a bit. I suspect it could be broken up a bit, maybe into
at least:
- EMIF interface
- SCM interface, new APIs
- assembly/OCM code
- pm33xx.[ch]
- lastly, the late_init stuff that actually initizlizes
Ok, I'll try splitting the patches in the next version.
I have a handful of comments below. I wrote this up on the plane over
the weekend, and I see that Santosh has already made some similar
comments, but I'll send mine anyways.
[...]
Doing this on every suspend looks a bit strange. Why not just have some
init function handle these devices once at boot. If this is really
needed on every suspend, it needs some more description, and probably
some basic stub drivers need to be created.
We do require it for every suspend (but more below). I'll add in more
description in the comment that's already there.
Also, if there are drivers for these devices, won't this interfere?
Hmm, I can think of the following scenarios
1. Runtime PM adapted drivers are compiled in - We'll have to ensure that
in their suspend callbacks they end up doing omap_hwmod_idle() via the
runtime PM APIs. In this case the omap_hwmod_enable() followed by
omap_hwmod_idle() is not necessary in the PM code.
2. The drivers are not compiled in - In this case, the hwmod code puts
the IPs in standby during bootup so the first suspend-resume iteration
will pass. During resuming, the SYSC configuration for forced standby will
be lost, so in the subsequent iterations these IPs won't go standby and the
hwmod code does not touch them since they never got enabled. In this case
we do need the sequence that is there currently.
3. For some reason the respective drivers don't idle the IPs during suspend -
(no_idle_on_suspend flag for the hwmod in DT?). In this scenario I think
we should abort the suspend process since we know that the suspend is not
going to work.
Is there some other scenario that needs to be covered?
How about adding some flag in hwmod code for handling this? Something
similar to what was done for MMC [1]. I think the problem that we have
is in some ways similar to the one addressed in [1].
+ /* Put the GFX clockdomains to sleep */
+ clkdm_sleep(gfx_l3_clkdm);
+ clkdm_sleep(gfx_l4ls_clkdm);
+
+ /* Try to put GFX to sleep */
+ pwrdm_set_next_pwrst(gfx_pwrdm, PWRDM_POWER_OFF);
ditto.
I'll check if this can be removed.
[...]
+static int am33xx_pm_begin(suspend_state_t state)
+{
+ int ret = 0;
+
+ disable_hlt();
+
+ /*
+* Physical resume address to be used by ROM code
+*/
+ wkup_m3-resume_addr = (AM33XX_OCMC_END - am33xx_do_wfi_sz +
+ am33xx_resume_offset + 0x4);
Why does this need to be calculated every suspend/resume?
Will move this to init phase.
+ wkup_m3-sleep_mode = IPC_CMD_DS0;
+ wkup_m3-ipc_data1 = DS_IPC_DEFAULT;
+ wkup_m3-ipc_data2 = DS_IPC_DEFAULT;
+
+ am33xx_ipc_cmd();
This IPC needs a cleaner interface/API. Also, since it involves
register writes to the SCM, it should be defined in control.c. (NOTE:
we're in the process of creating a real driver out of the SCM, so all
SCM register accesses need to be contained in control.c)
For example, you probably want an am33xx_m3_* API in control.c, with
some pre-baked commands for the M3.
Ok. I'll work on this for the next version.
+ wkup_m3-state = M3_STATE_MSG_FOR_LP;
+ omap_mbox_enable_irq(wkup_m3-mbox, IRQ_RX);
+
+ ret = omap_mbox_msg_send(wkup_m3-mbox, 0xABCDABCD);
+ if (ret) {
+ pr_err(A8-CM3 MSG for LP failed\n);
+ am33xx_m3_state_machine_reset();
+ ret = -1;
+ }
+
+ if (!wait_for_completion_timeout(wkup_m3_sync,
+ msecs_to_jiffies(500))) {
hmm, interesting. I know you're not implementing idle here, but I'm
rather curious how this sync w/M3 is going to work for idle.
Like you mentioned in another thread we could probably not have
a sync in the idle path. (More in the other thread).
+ pr_err(A8-CM3 sync failure\n);
+ am33xx_m3_state_machine_reset();
+ ret = -1;
+ } else {
+ pr_debug(Message sent for entering DeepSleep mode\n);
+ omap_mbox_disable_irq(wkup_m3-mbox, IRQ_RX);
+ }
+
+ return ret;
+}
+
[...]
+static void am33xx_m3_state_machine_reset(void)
+{
+ int ret = 0;
+
+ wkup_m3-resume_addr= 0x0;
+ wkup_m3-sleep_mode = IPC_CMD_RESET;
+ wkup_m3-ipc_data1 = DS_IPC_DEFAULT;
+ wkup_m3-ipc_data2 = DS_IPC_DEFAULT;
+
+ am33xx_ipc_cmd();
+
+ wkup_m3-state = M3_STATE_MSG_FOR_RESET;
+
+ ret = omap_mbox_msg_send(wkup_m3-mbox, 0xABCDABCD);
magic constant needs a #define
Ok.
+ if (!ret) {
+
RE: [PATCH 15/15] ARM: OMAP2+: AM33XX: Basic suspend resume support
Hi Santosh, Kevin On Tue, Nov 06, 2012 at 03:22:16, Shilimkar, Santosh wrote: [...] + +/* + * This a subset of registers defined in drivers/memory/emif.h + * Move that to include/linux/? + */ I'd probably suggest just moving the register definitions you need into plat/emif_plat.h so they can be shared with the driver. Also, the EMIF stuff would benefit greatly from using symbolic defines for the values being written. Probably having those in plat/emif_plat.h would also help out here. Or, maybe the EMIF driver can provide some self-contained stubs that can be copied to OCP RAM for the functionality needed here? Santosh, what do you think of that? Thats what I mentioned in my comment. I also don't know why such a bad hardware choice was made when we have perfectly working EMIF IP across low power states. Even the self refresh control is managed inside hardware upon idle. My guess is DDR self-refresh might be the reason to use OCMC RAM. In either case, Keeping EMIF changes separate as part of EMIF driver/platform code is right way to go about it. May be the disable_selfrefresh() might need to kept in assembly files since it needs to be running from SRAM but rest need not be part of PM code. In the suspend path we do lot of I/O manipulations to lower final power number which must be done after the external memory has gone into self-refresh. So, these steps will have to be done from code in OCMC RAM. Other than saving the EMIF configuration the other thing that we do during suspend from assembly is to put the PLLs in bypass. We could possibly move the DISP PLL bypass to C code. MPU PLL is another candidate but this might add in more delays in the suspend and abort sequence (I don't have any delay numbers to justify this right now) The resume path undoes the I/O manipulations and then restores the EMIF configuration all of which I think is necessary before we can jump back to external memory. So, I think we'll have just the EMIF context save and possibly the PLL bypass for DISP PLL during suspend that we can move out of assembly. Coming to point of sharing the EMIF register definitions, with the recent changes to move around the header files out of plat-omap, is it ok to add in the required offsets and related bit-field definitions from the EMIF driver to plat-omap? Regards, Vaibhav -- 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 15/15] ARM: OMAP2+: AM33XX: Basic suspend resume support
On Tue, Nov 06, 2012 at 18:08:36, Shilimkar, Santosh wrote: On Tuesday 06 November 2012 06:29 AM, Bedia, Vaibhav wrote: Hi Santosh, Kevin On Tue, Nov 06, 2012 at 03:22:16, Shilimkar, Santosh wrote: [...] + +/* + * This a subset of registers defined in drivers/memory/emif.h + * Move that to include/linux/? + */ I'd probably suggest just moving the register definitions you need into plat/emif_plat.h so they can be shared with the driver. Also, the EMIF stuff would benefit greatly from using symbolic defines for the values being written. Probably having those in plat/emif_plat.h would also help out here. Or, maybe the EMIF driver can provide some self-contained stubs that can be copied to OCP RAM for the functionality needed here? Santosh, what do you think of that? Thats what I mentioned in my comment. I also don't know why such a bad hardware choice was made when we have perfectly working EMIF IP across low power states. Even the self refresh control is managed inside hardware upon idle. My guess is DDR self-refresh might be the reason to use OCMC RAM. In either case, Keeping EMIF changes separate as part of EMIF driver/platform code is right way to go about it. May be the disable_selfrefresh() might need to kept in assembly files since it needs to be running from SRAM but rest need not be part of PM code. In the suspend path we do lot of I/O manipulations to lower final power number which must be done after the external memory has gone into self-refresh. So, these steps will have to be done from code in OCMC RAM. Only the DDR IO needs to be done after memory enters into self refresh. Rest of the IOs can be handled and moved to low power modes before DDR being in self refresh, No ? All the code is related to DDR IOs only. We have some code for changing the pull configuration of the DATA and CMD macros of the PHY and then some code for DDR VTP control. We also switch over the DDR IOs to mDDR mode to lower the leakage. Other than saving the EMIF configuration the other thing that we do during suspend from assembly is to put the PLLs in bypass. We could possibly move the DISP PLL bypass to C code. MPU PLL is another candidate but this might add in more delays in the suspend and abort sequence (I don't have any delay numbers to justify this right now) So DPLLS doesn't support low power bypass mode which should take care of itself on clock gating ? Are the DPLL IPs different than what they are on OMAP ? Same IP but no auto-mode :( The resume path undoes the I/O manipulations and then restores the EMIF configuration all of which I think is necessary before we can jump back to external memory. As I memtioned above, you should limit these IOs to only DDR IOs. So, I think we'll have just the EMIF context save and possibly the PLL bypass for DISP PLL during suspend that we can move out of assembly. EMIF context save also can be done in advance. Restore is what you need to take care in early resume before bringing out DDR out of self refresh. Yes I can move the context save earlier. Will try that out and put in the next version. Coming to point of sharing the EMIF register definitions, with the recent changes to move around the header files out of plat-omap, is it ok to add in the required offsets and related bit-field definitions from the EMIF driver to plat-omap? We can have that in linux/include/* as well if the register defines can be shared. Ok. Regards, Vaibhav -- 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 08/15] ARM: OMAP2+: hwmod: Fix the omap_hwmod_addr_space for CPGMAC0
On Tue, Nov 06, 2012 at 18:38:08, Hiremath, Vaibhav wrote: On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote: On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote: [...] Ok I checked this one. The change I made was indirectly fixing another issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two entries and the SYSC register is part of the second entry. The function _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with the flag ADDR_TYPE_RT flag. The change I made indirectly made the second entry in am33xx_cpgmac0_addr_space[] become the first memory space with the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct SYSC address of CPGMAC0 and the IP went to standby during bootup. After changing the order of the entries in am33xx_cpgmac0_addr_space[] things work fine. Good catch. Just a side note on this, driver expects the addresses in this order only, first SS and then WR. Sorry I didn't understand your comment. For HWMOD code to work as expected, we need to change the order. Why do you want to change the order? Just specify ADDR_TYPE_RT, that should be it. The problem is that the memory space without the SYSC comes first and is labeled as ADDR_TYPE_RT. I think this is not correct and hence either we change the order or remove the flag from the first entry. If we do the latter then taking the logic of putting in the flag only for memory spaces with SYSC further we need to fixup the entries for ephrpwm0/1/2 and ecap0/1/2. Regards, Vaibhav -- 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 08/15] ARM: OMAP2+: hwmod: Fix the omap_hwmod_addr_space for CPGMAC0
Hi Benoit, On Tue, Nov 06, 2012 at 19:20:46, Cousson, Benoit wrote: Hi Vaibhav Vaibhav, On 11/06/2012 02:46 PM, Bedia, Vaibhav wrote: On Tue, Nov 06, 2012 at 18:38:08, Hiremath, Vaibhav wrote: On Tue, Nov 06, 2012 at 15:39:14, Bedia, Vaibhav wrote: On Tue, Nov 06, 2012 at 14:59:45, Hiremath, Vaibhav wrote: [...] Ok I checked this one. The change I made was indirectly fixing another issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two entries and the SYSC register is part of the second entry. The function _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with the flag ADDR_TYPE_RT flag. The change I made indirectly made the second entry in am33xx_cpgmac0_addr_space[] become the first memory space with the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct SYSC address of CPGMAC0 and the IP went to standby during bootup. After changing the order of the entries in am33xx_cpgmac0_addr_space[] things work fine. Good catch. Just a side note on this, driver expects the addresses in this order only, first SS and then WR. Sorry I didn't understand your comment. For HWMOD code to work as expected, we need to change the order. Why do you want to change the order? Just specify ADDR_TYPE_RT, that should be it. The problem is that the memory space without the SYSC comes first and is labeled as ADDR_TYPE_RT. I think this is not correct and hence either we change the order or remove the flag from the first entry. If we do the latter then taking the logic of putting in the flag only for memory spaces with SYSC further we need to fixup the entries for ephrpwm0/1/2 and ecap0/1/2. The order should not matter, just use ADDR_TYPE_RT for the relevant entry. I have a patch ongoing to remove this flag for the non-SYSC entry to avoid this kind of confusion. The name should probably be changed as well to reflect that at some point. Since these entries will be removed anyway with pure DT boot, that should be a temporary issue. Thanks for the clarification. I'll make the change accordingly. Regards, Vaibhav -- 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 11/15] ARM: OMAP: timer: Interchange clksrc and clkevt for AM33XX
Hi Kevin, On Mon, Nov 05, 2012 at 23:33:07, Kevin Hilman wrote: Bedia, Vaibhav vaibhav.be...@ti.com writes: On Sat, Nov 03, 2012 at 18:34:30, Kevin Hilman wrote: [...] Doesn't this also mean that you won't get timer wakeups in idle? Or are you keeping the domain where the clockevent is on during idle? The lowest idle state that we are targeting will have MPU powered off with external memory in self-refresh mode. Peripheral domain with the clockevent will be kept on. Is this a limitation of the hardware? or the software? Well, making the lowest idle state same as the suspend state will require us to involve WKUP_M3 in the idle path and wakeup sources get limited to the IPs in the WKUP domain alone. There's no IO daisy chaining in AM33XX so that's one big difference compared to OMAP. The other potential problem is that the IPC mechanism that we have uses interrupts. It can still interrupt the M3, it's only the interrupt back to the MPU that is the issue, right? That being said, there's no reason it couldn't use polling in the idle path, right? Yes we could use polling but I think we have a bigger problem in the chip architecture. Assuming that the lowest idle state, say Cx, is the same as the suspend state, we'll need to communicate with the WKUP_M3 using interrupts once we decide to enter Cx. I am not sure if we can do something in the cpuidle implementation to work around the interrupt for idle problem. We could probably not wait for an ACK when we want to enter Cx, why not? Are the response times from the M3 really up to 500ms (guessing based on the timeout you used in the suspend path.) That seems rather unlikely. No 500ms is too high. Actual delays would be much lower, I need to check with the design team on the expected number. Hmm, but as I think about it. Why does the MPU need to wait for an ACK at all? Why not just send the cmd and WFI? I have myself being going back and forth on this. There are lot of things that we do in software, DDR being one of them. We can't do some of the DDR related stuff unless memory enter self-refresh AND EMIF gets disabled. Doing so essentially means that the drivers have entered sort of suspend state. Given this h/w limitation I don't see how we could handle without impacting a running system. but the problem of limited wakeup sources remains. If we let the various drivers block the entry to Cx, since almost all the IPs are in the peripheral domain a system which uses anything other than UART and Timer in WKUP domain will probably never be able enter Cx. Even so, I think the system needs to be designed to hit the same power states in idle and suspend. Then, the states can be restricted based wakeup capabilities as you described. This would be easy to do in the runtime PM implementation for this device. IMO, assuming that idle will not be useful from the begining is leading down the path to poor design choices that will be much more difficult to fixup down the road in order to add idle support later. We need to design both idle and suspend at the same time. Getting PER to transition on a running system is something I can't figure out. Maybe MPU OFF is the lowest we can go. Also, don't forget about GPIO0. Systems could easily be built such that peripherals which want to wakeup but don't have native wakeup capabilities could use a GPIO in bank 0 to wake the system. Similarily, I2C0 is in WKUP, and brought out to capes, so some simple designs with with I2C devices on a cape might be perfectly capable of hitting deep power states in idle. Ok this is interesting. AFAIK I2C wakeup requires the device to be operating in slave mode. If so, is this something that's already supported on OMAP? Regards, Vaibhav -- 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 11/15] ARM: OMAP: timer: Interchange clksrc and clkevt for AM33XX
Hi Santosh, On Tue, Nov 06, 2012 at 03:29:22, Shilimkar, Santosh wrote: [...] IMO, assuming that idle will not be useful from the begining is leading down the path to poor design choices that will be much more difficult to fixup down the road in order to add idle support later. We need to design both idle and suspend at the same time. I agree with Kevin. Not supporting CPUIDLE deep states can hit the active power numbers dearly. I just don't know why the SOCs don't share the standard and must have design choices. But thats another discussion. Yes, active power numbers are not comparable to OMAP :( How about leaving the timer choices as is. PER timer for clock source and wakeuptimer for clock event. Anyway in suspend the clock-source can be suspended and that is evident from recent discussion. The only downside is you won't count time in suspend which is any way the case. Vaibhav, Do you guys see any implementation bottleneck for above ? Looking at the timekeeping code I see one more potential reason for making this change. OMAP registers the 32k sync timer as the persistent clock and since there's no 32k sync timer in AM33xx it doesn't register a persistent clock right now. Based on what I understood, we need to have to register one and DMTimer1 is the only clock that can serve as the persistent clock in suspend state. When we do so we might as well use it as the clocksource. A related question that I had was, is there a mechanism to handle the 32k counter (DMTimer or sync timer) wraparound condition in suspend? Regards, Vaibhav -- 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 08/15] ARM: OMAP2+: hwmod: Fix the omap_hwmod_addr_space for CPGMAC0
On Sun, Nov 04, 2012 at 20:54:17, Bedia, Vaibhav wrote: On Sat, Nov 03, 2012 at 21:48:48, Shilimkar, Santosh wrote: On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote: The first entry for CPGMAC0 should be ADDR_MAP_ON_INIT instead of ADDR_TYPE_RT to ensure the omap hwmod code maps the memory space at init and writes to the SYSCONFIG registers. Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com --- Sorry again similar question. Why CPGMAC0 should be mapped and sysconfig updated early ? Hmm I need to revisit this one. CPGMAC0 was not going to standby without this. Maybe something else is wrong in the hwmod data and needs fixing. Ok I checked this one. The change I made was indirectly fixing another issue with the AM33xx hwmod data. am33xx_cpgmac0_addr_space[] has two entries and the SYSC register is part of the second entry. The function _find_mpu_rt_addr_space in omap_hwmod.c looks for the first entry with the flag ADDR_TYPE_RT flag. The change I made indirectly made the second entry in am33xx_cpgmac0_addr_space[] become the first memory space with the ADDR_TYPE_RT flag. Due to this the hwmod code wrote to the correct SYSC address of CPGMAC0 and the IP went to standby during bootup. After changing the order of the entries in am33xx_cpgmac0_addr_space[] things work fine. I'll make the changes in the next version. Regards, Vaibhav -- 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 06/15] ARM: OMAP2+: hwmod: Enable OCMCRAM registration in AM33XX
On Mon, Nov 05, 2012 at 12:53:59, Hiremath, Vaibhav wrote: Can you cut-n-paste the ocmcram hwmod entry outside of #if and resubmit it again? Ok. Will do that in the next version. Regards, Vaibhav -- 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 13/15] ARM: DTS: AM33XX: Add nodes for OCMCRAM and Mailbox
On Mon, Nov 05, 2012 at 20:23:11, Shilimkar, Santosh wrote: [...] On OMAP the OCMC RAM is always clocked and doesn't need any special clock enable. CM_L3_2_OCMC_RAM_CLKCTRL module mode field is read only. Isn't it same on AMXX ? On AM33xx, OCMC RAM is in PER domain and the corresponding CLKCLTR module mode fields are r/w. OCMC RAM needs to be disabled as part of the DeepSleep0 entry to let PER domain transition. Regards, Vaibhav -- 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 04/15] ARM: OMAP2+: hwmod: Update the reset API for AM33XX
On Mon, Nov 05, 2012 at 12:28:36, Hiremath, Vaibhav wrote: [...] - u32 mask = 1 shift; - - /* Check the current status to avoid de-asserting the line twice */ - if (am33xx_prm_is_hardreset_asserted(shift, inst, rstctrl_offs) == 0) - return -EEXIST; Any specific reason why you have removed this check? During bootup the hardreset line is asserted, so wouldn't that check lead to the function always returning without doing anything? Regards, Vaibhav -- 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 13/15] ARM: DTS: AM33XX: Add nodes for OCMCRAM and Mailbox
On Tue, Nov 06, 2012 at 03:15:10, Shilimkar, Santosh wrote: On Tuesday 06 November 2012 02:49 AM, Santosh Shilimkar wrote: On Tuesday 06 November 2012 12:59 AM, Kevin Hilman wrote: Bedia, Vaibhav vaibhav.be...@ti.com writes: On Mon, Nov 05, 2012 at 20:23:11, Shilimkar, Santosh wrote: [...] On OMAP the OCMC RAM is always clocked and doesn't need any special clock enable. CM_L3_2_OCMC_RAM_CLKCTRL module mode field is read only. Isn't it same on AMXX ? On AM33xx, OCMC RAM is in PER domain and the corresponding CLKCLTR module mode fields are r/w. OCMC RAM needs to be disabled as part of the DeepSleep0 entry to let PER domain transition. After DeepSleep0, the ROM code is being given an address in OCMC RAM to jump to. If OCMC RAM is disabled as part of suspend, this means that OCMC RAM contents are maintained even though PER domain transitions? If so, that needs to be more clearly documented. Thats very good point. How does OCMC RAM retains the contents without clock ? Ignore the question. I figured out from other patch changelog the OCMC RAM supports retention. Please have that clearly captured in change log. Yes, OCMC RAM support retention. Will document that here also. Regards, Vaibhav -- 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/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
[...]
+#define SHUTDOWN_TIME_SEC2
+#define SECS_IN_MIN 60
+#define WAIT_AFTER (SECS_IN_MIN - SHUTDOWN_TIME_SEC)
+#define WAIT_TIME_MS (SHUTDOWN_TIME_SEC * 1000)
+
static void __iomem *rtc_base;
[...]
+
+ /* Wait few seconds instead of rollover */
+ do {
+ omap_rtc_read_time(NULL, tm);
+ if (WAIT_AFTER = tm.tm_sec)
+ mdelay(WAIT_TIME_MS);
+ } while (WAIT_AFTER = tm.tm_sec);
This hardcoded wait for rollover doesn't look good. I see some
helper functions in rtc-lib.c which probably could be used for
converting the current time to elapsed seconds, add the delay and
then convert it back to the time to be programmed in RTC without
worrying about rollover. Why not use that?
+
+ /* Add shutdown time to the current value */
+ tm.tm_sec += SHUTDOWN_TIME_SEC;
+
+ if (tm2bcd(tm) 0)
+ return;
+
+ pr_info(System will go to power_off state in approx. %d secs\n,
+ SHUTDOWN_TIME_SEC);
+
+ /* Set the ALARM2 time */
+ rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
+ rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
+ rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
+ rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
+ rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
+ rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
+
+ /* Enable alarm2 interrupt */
+ val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
+ writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
+ rtc_base + OMAP_RTC_INTERRUPTS_REG);
+
These registers are not present in older versions of the IP so how
does that get handled?
You also need to describe the connection between the ALARM2 and the
power off logic in detail.
Regards,
Vaibhav
--
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/8] PWMSS: Add PWM Subsystem driver for parent-child relationship
On Mon, Nov 05, 2012 at 14:42:22, Philip, Avinash wrote:
[...]
+pwmss0: pwmss@4830 {
+ compatible = ti,am33xx-pwmss;
+ reg = 0x4830 0x10
+ 0x48300100 0x80
+ 0x48300180 0x80
+ 0x48300200 0x80;
Do you really need the 4 address ranges here? You eventually do add in
child nodes with other address ranges so isn't the first entry sufficient?
I haven't really looked at the DT details so it is enforced by that let me
know.
[...]
+
+#define PWMSS_CLKCONFIG 8
+
This #def can use a comment.
+void pwmss_submodule_state_change(struct device *dev, int pos, bool enable)
+{
+ struct pwmss_info *info = dev_get_drvdata(dev);
+ u16 val;
+
+ val = readw(info-mmio_base + PWMSS_CLKCONFIG);
+ if (enable)
+ val |= 1 pos;
+ else
+ val = ~(1 pos);
+ mutex_lock(info-pwmss_lock);
+ writew(val , info-mmio_base + PWMSS_CLKCONFIG);
+ mutex_unlock(info-pwmss_lock);
+}
+EXPORT_SYMBOL(pwmss_submodule_state_change);
I see a clk_en_ack field in the clock status register. You should be checking
that.
[...]
+
+MODULE_DESCRIPTION(pwmss driver);
+MODULE_AUTHOR(Texas Instruments);
+MODULE_LICENSE(GPL);
diff --git a/drivers/pwm/tipwmss.h b/drivers/pwm/tipwmss.h
new file mode 100644
index 000..83fdc29
--- /dev/null
+++ b/drivers/pwm/tipwmss.h
@@ -0,0 +1,8 @@
License text?
+#ifdef CONFIG_PWM_TIPWMSS
+extern void pwmss_submodule_state_change(struct device *dev, int pos,
+ bool enable);
+#else
+static inline void pwmss_submodule_state_change(struct device *dev, int pos,
+ bool enable)
+{}
+#endif
Regards,
Vaibhav
--
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 8/8] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
On Mon, Nov 05, 2012 at 14:42:29, Philip, Avinash wrote:
[...]
+ am33xx_pinmux: pinmux@44e10800 {
+ ecap0_pins: backlight_pins {
+ pinctrl-single,pins =
+ 0x164 0x0 /*
eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
+ ;
+ };
+ };
+
Ok I see the pinctrl entries here. Shouldn't this be done before
the change in the driver?
Regards.
Vaibhav
--
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 6/8] pwm: pwm-tiehrpwm: Adding TBCLK gating support.
On Mon, Nov 05, 2012 at 14:42:27, Philip, Avinash wrote:
[...]
+ /* Some platforms require explicit tbclk gating */
+ if (of_property_read_bool(pdev-dev.of_node, tbclkgating)) {
+ pc-tbclk = clk_get(pdev-dev, tbclk);
+ if (IS_ERR(pc-tbclk)) {
+ dev_err(pdev-dev, Could not get EHRPWM TBCLK\n);
+ return PTR_ERR(pc-tbclk);
+ }
+ }
+
+ /* Enable tbclk leave */
+ if (pc-tbclk)
+ clk_enable(pc-tbclk);
+
Here also why are you leaving this clock always running?
Regards,
Vaibhav
--
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 5/8] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
On Mon, Nov 05, 2012 at 14:42:26, Philip, Avinash wrote:
[...]
+#include linux/of_device.h
+#include linux/pinctrl/consumer.h
Pinctrl changes should be separate patch. Morevoer, you don't mention
that you making this change.
+
+#include tipwmss.h
/* EHRPWM registers and bits definitions */
@@ -107,6 +111,10 @@
#define AQCSFRC_CSFA_FRCHIGH BIT(1)
#define AQCSFRC_CSFA_DISSWFRC(BIT(1) | BIT(0))
+#define EPWMCLK_EN_SHIFT 8
+
+#define PWM_CELL_SIZE3
+
#define NUM_PWM_CHANNEL 2 /* EHRPWM channels */
struct ehrpwm_pwm_chip {
@@ -392,12 +400,27 @@ static const struct pwm_ops ehrpwm_pwm_ops = {
.owner = THIS_MODULE,
};
+#ifdef CONFIG_OF
+static const struct of_device_id ehrpwm_of_match[] = {
+ {
+ .compatible = ti,am33xx-ehrpwm,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ehrpwm_of_match);
+#endif
+
static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
{
int ret;
struct resource *r;
struct clk *clk;
struct ehrpwm_pwm_chip *pc;
+ struct pinctrl *pinctrl;
+
+ pinctrl = devm_pinctrl_get_select_default(pdev-dev);
I didn't see a patch adding the pinctrl entries.
+ if (IS_ERR(pinctrl))
+ dev_warn(pdev-dev, failed to configure pins from driver\n);
pc = devm_kzalloc(pdev-dev, sizeof(*pc), GFP_KERNEL);
if (!pc) {
@@ -419,6 +442,7 @@ static int __devinit ehrpwm_pwm_probe(struct
platform_device *pdev)
pc-chip.dev = pdev-dev;
pc-chip.ops = ehrpwm_pwm_ops;
+ pc-chip.of_pwm_n_cells = PWM_CELL_SIZE;
pc-chip.base = -1;
pc-chip.npwm = NUM_PWM_CHANNEL;
@@ -437,8 +461,11 @@ static int __devinit ehrpwm_pwm_probe(struct
platform_device *pdev)
dev_err(pdev-dev, pwmchip_add() failed: %d\n, ret);
return ret;
}
-
pm_runtime_enable(pdev-dev);
+ pm_runtime_get_sync(pdev-dev);
+ pwmss_submodule_state_change(pdev-dev.parent, EPWMCLK_EN_SHIFT, true);
I think you should modify this API to return the status for drivers to check.
Another related question, why should this clock be enabled in probe and not
only when it
is required? Shouldn't it be disabled in suspend?
Regards,
Vaibhav
--
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 04/15] ARM: OMAP2+: hwmod: Update the reset API for AM33XX
On Tue, Nov 06, 2012 at 11:36:20, Hiremath, Vaibhav wrote: [...] The code is checking whether the line is already de-asserted (== 0), so I am not sure how this will change if hardreset line is asserted during bootup. You are right. I just checked the behavior since I recall seeing something odd earlier. Looks like this is needed to avoid issues with subsequent hardreset deassertions so I'll put it back in. Regards. Vaibhav -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Jon,
On Tue, Nov 06, 2012 at 02:34:05, Hunter, Jon wrote:
[...]
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
So these suspend/resume callbacks are going to be called for all OMAP2+
and AM devices? I don't think we want that. AFAIK OMAP timers will
idle on their own when stopped and don't require this.
IMO instead of skipping the callback registration we could have checks in the
suspend/resume callbacks to decide what to do.
I'll check if the idling part is AM33xx specific. If not, based on the recent
timer
changes that you did, perhaps checking if the clockevent selected doesn't have
the
ti,timer-alwon capability will be good enough. What do you think?
Regards.
Vaibhav
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Santosh,
On Sat, Nov 03, 2012 at 21:22:04, Shilimkar, Santosh wrote:
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
Actually OMAP also uses only one timer. The clocksource
is taken care by 32K syntimer till OMAP4 and by realtime
counter on OMAP5. There is a clocksource registration of
timer is available but that is not being used in systems.
Yes, I guess the changelog should mention that AM33xx does not
have the 32k synctimer. I'll also add in the OMAP details that
you pointed out so that all the details get captured.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
So you use WKUP domain timer for clocksource and PER
domain one for clock-event ?
Yes, that's correct.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
You can move all the look up stuff in init code and then
suspend resume hooks will be cleaner.
Will do. Kevin also pointed this out.
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
OK. So since your clk_event stops when PER idles, how do you plan
to support the SOC idle. For CPUIDLE path, you need your clock-event
to wakeup the system based on next timer expiry. So you need your
clock event to be active. Indirectly, you can't let PER idle which
leads npo CORE idle-SOC idle.
How do you plan to address this ? Os is SOC idle is not suppose
to be added for AMXXX ?
We can't really have SOC idle on AM33xx or at least that's what I think.
The deepest that we should be able to support is MPU off with external
memory in self-refresh mode. I mentioned the reasons for that in the
reply to Kevin [1]. If there's any another approach that we could take
that would be great to know.
Regards,
Vaibhav
[1] http://marc.info/?l=linux-arm-kernelm=135195053104053w=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
