Re: [PATCH 0/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
On Thu, 16 Jul 2015, Tero Kristo wrote: On 07/16/2015 03:15 AM, Paul Walmsley wrote: On Tue, 14 Jul 2015, Tero Kristo wrote: On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh [...] It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. I don't see the race. Where is it? See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock. That code is accessing the exact same registers. I guess my question is, when is it possible that code could race with the hwmod code for the same device? - Paul -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
On 07/16/2015 03:15 AM, Paul Walmsley wrote: On Tue, 14 Jul 2015, Tero Kristo wrote: On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh [...] It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. I don't see the race. Where is it? See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock. That code is accessing the exact same registers. -Tero -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
On 07/16/2015 01:13 PM, Paul Walmsley wrote: On Thu, 16 Jul 2015, Tero Kristo wrote: On 07/16/2015 03:15 AM, Paul Walmsley wrote: On Tue, 14 Jul 2015, Tero Kristo wrote: On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh [...] It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. I don't see the race. Where is it? See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock. That code is accessing the exact same registers. I guess my question is, when is it possible that code could race with the hwmod code for the same device? Hmm yea I think you are right, this only gets potentially called within pm_runtime_get/put_sync for RTC. The current sequence is highly inefficient though, as we are doing multiple lock/unlock operations to the RTC from multiple sources. See following rtcwake trace on am43xx-gp-evm as an example. / # rtcwake -s 4 -m mem [7.425322] am3352_rtc_unlock [7.428330] am3352_rtc_lock [7.431139] am3352_rtc_unlock [7.434116] am3352_rtc_lock wakeup from mem at Sat Jan 1 00:00:11 2000 [7.448549] PM: Syncing filesystems ... done. [7.455425] Freezing user space processes ... (elapsed 0.001 seconds) done. [7.463738] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) do ne. [7.472532] Suspending console(s) (use no_console_suspend to debug) [7.481878] am3352_rtc_unlock [7.481889] am3352_rtc_lock [7.482307] PM: suspend of devices complete after 2.713 msecs [7.483479] PM: late suspend of devices complete after 1.153 msecs [7.484727] omap_hwmod_rtc_unlock [7.484733] omap_hwmod_rtc_lock [7.485182] PM: noirq suspend of devices complete after 1.685 msecs [7.485190] Disabling non-boot CPUs ... [7.485199] PM: Successfully put all powerdomains to target state [7.485199] PM: Wakeup source RTC Alarm [7.499853] PM: noirq resume of devices complete after 14.558 msecs [7.500047] am3352_rtc_unlock [7.500052] am3352_rtc_lock [7.500123] am3352_rtc_unlock [7.500128] am3352_rtc_lock [7.501019] PM: early resume of devices complete after 0.809 msecs [7.501464] am3352_rtc_unlock [7.501472] am3352_rtc_lock [7.558046] PM: resume of devices complete after 57.007 msecs [7.638807] Restarting tasks ... done. [7.643173] am3352_rtc_unlock [7.646162] am3352_rtc_lock But, I guess this is for some interested party to optimize if needed, and it is mostly an issue with the RTC driver itself. -Tero -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
Hi Tero, On Thursday 16 July 2015 05:33 PM, Tero Kristo wrote: On 07/16/2015 01:13 PM, Paul Walmsley wrote: On Thu, 16 Jul 2015, Tero Kristo wrote: On 07/16/2015 03:15 AM, Paul Walmsley wrote: On Tue, 14 Jul 2015, Tero Kristo wrote: On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh [...] It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. I don't see the race. Where is it? See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock. That code is accessing the exact same registers. I guess my question is, when is it possible that code could race with the hwmod code for the same device? Hmm yea I think you are right, this only gets potentially called within pm_runtime_get/put_sync for RTC. Yes, sysc is written from hwmod_init code and pm_runtime_get/put_sysnc. And we write into rtc registers only after pm_runtime_get_sync and rtc_unlock. There cannot be a race condition here. The current sequence is highly inefficient though, as we are doing multiple lock/unlock operations to the RTC from multiple sources. See following rtcwake trace on am43xx-gp-evm as an example. Initially I had a patch which leaves rtc unlocked at hwmod_init instead of doing unlock and lock for each set of register writes in the driver. But it was rejected stating that deviates the purpose of locking mechanism. So I updated the driver for adapting this locking and unlocking mechanism. Thanks and regards, Lokesh / # rtcwake -s 4 -m mem [7.425322] am3352_rtc_unlock [7.428330] am3352_rtc_lock [7.431139] am3352_rtc_unlock [7.434116] am3352_rtc_lock wakeup from mem at Sat Jan 1 00:00:11 2000 [7.448549] PM: Syncing filesystems ... done. [7.455425] Freezing user space processes ... (elapsed 0.001 seconds) done. [7.463738] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) do ne. [7.472532] Suspending console(s) (use no_console_suspend to debug) [7.481878] am3352_rtc_unlock [7.481889] am3352_rtc_lock [7.482307] PM: suspend of devices complete after 2.713 msecs [7.483479] PM: late suspend of devices complete after 1.153 msecs [7.484727] omap_hwmod_rtc_unlock [7.484733] omap_hwmod_rtc_lock [7.485182] PM: noirq suspend of devices complete after 1.685 msecs [7.485190] Disabling non-boot CPUs ... [7.485199] PM: Successfully put all powerdomains to target state [7.485199] PM: Wakeup source RTC Alarm [7.499853] PM: noirq resume of devices complete after 14.558 msecs [7.500047] am3352_rtc_unlock [7.500052] am3352_rtc_lock [7.500123] am3352_rtc_unlock [7.500128] am3352_rtc_lock [7.501019] PM: early resume of devices complete after 0.809 msecs [7.501464] am3352_rtc_unlock [7.501472] am3352_rtc_lock [7.558046] PM: resume of devices complete after 57.007 msecs [7.638807] Restarting tasks ... done. [7.643173] am3352_rtc_unlock [7.646162] am3352_rtc_lock But, I guess this is for some interested party to optimize if needed, and it is mostly an issue with the RTC driver itself. -Tero -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
On Tue, 14 Jul 2015, Tero Kristo wrote: On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh [...] It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. I don't see the race. Where is it? - Paul -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh Tested on: DRA7-evm: http://pastebin.ubuntu.com/1169/ DRA72-evm: http://pastebin.ubuntu.com/11688901/ BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/ BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/ AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC) Lokesh Vutla (3): ARM: OMAP2+: hwmod: add support for lock and unlock hooks ARM: DRA: hwmod: RTC: Add lock and unlock functions ARM: AMx3xx: RTC: Add lock and unlock functions arch/arm/mach-omap2/omap_hwmod.c | 13 ++ arch/arm/mach-omap2/omap_hwmod.h | 6 +++ .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_reset.c | 47 ++ 5 files changed, 70 insertions(+) -- 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/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
On 07/14/2015 01:09 PM, Lokesh Vutla wrote: Hi, On Wednesday 10 June 2015 02:56 PM, Lokesh Vutla wrote: Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. ping on this series. Thanks and regards, Lokesh This looks kind of hackish to have the unlock + lock functionality copy pasted to both driver and hwmod. It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver. Can we avoid these issues somehow? -Tero Tested on: DRA7-evm: http://pastebin.ubuntu.com/1169/ DRA72-evm: http://pastebin.ubuntu.com/11688901/ BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/ BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/ AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC) Lokesh Vutla (3): ARM: OMAP2+: hwmod: add support for lock and unlock hooks ARM: DRA: hwmod: RTC: Add lock and unlock functions ARM: AMx3xx: RTC: Add lock and unlock functions arch/arm/mach-omap2/omap_hwmod.c | 13 ++ arch/arm/mach-omap2/omap_hwmod.h | 6 +++ .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_reset.c | 47 ++ 5 files changed, 70 insertions(+) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks
Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register. And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly. Tested on: DRA7-evm: http://pastebin.ubuntu.com/1169/ DRA72-evm: http://pastebin.ubuntu.com/11688901/ BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/ BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/ AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC) Lokesh Vutla (3): ARM: OMAP2+: hwmod: add support for lock and unlock hooks ARM: DRA: hwmod: RTC: Add lock and unlock functions ARM: AMx3xx: RTC: Add lock and unlock functions arch/arm/mach-omap2/omap_hwmod.c | 13 ++ arch/arm/mach-omap2/omap_hwmod.h | 6 +++ .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 + arch/arm/mach-omap2/omap_hwmod_reset.c | 47 ++ 5 files changed, 70 insertions(+) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
