On 2018-08-30 12:45, Krzysztof Kozlowski wrote: > On Thu, 30 Aug 2018 at 12:34, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: >> Hi Krzysztof, >> >> On 2018-08-30 12:25, Krzysztof Kozlowski wrote: >>> On Thu, 30 Aug 2018 at 11:59, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: >>>> On 2018-08-30 08:29, Krzysztof Kozlowski wrote: >>>>> On Wed, 29 Aug 2018 at 18:00, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: >>>>>> Clocks should be suspended as late as possible and resumed as early as >>>>>> possible to let other drivers do their own suspend/resume tasks. NOIRQ >>>>>> callbacks better suit this requirement. >>>>> I think that's not a good reason to use the noirq versions. These are >>>>> to solve the races with interrupt handlers, not to manually order >>>>> callbacks. >>>> Then please tell me which other solution should I use to make clock >>>> available >>>> on Exynos5433 during NOIRQ suspend/resume phase. dw-mmc driver requires to >>>> access its clocks in NOIRQ resume. >>> Indeed I found the usage of noirq in the dw-mmc driver which made me >>> wondering why it is there... and if you look at explanation, the noirq >>> is only for the purpose of clearing wakeup interrupt in CLKSEL >>> register. >>> >>> Further code refactoring moved more and more code to suspend_noirq, >>> including the runtime PM part. This probably should not be part of >>> suspend_noirq but regular suspend. Then all the need of manual >>> ordering would go away. So the answer to your question - try fixing >>> buggy dw-mmc driver. :) >> It is not the bug in dw-mmc driver. It clearly needs to access some its >> registers during NOIRQ phase. However accessing registers requires to >> have clocks enabled, which is being handled by runtime PM. There is >> nothing broken here. > Ah I missed that point and I see you fixed that case in "mmc: > dw_mmc-exynos: fix potential external abort in resume_noirq()". The > true reasoning for this patch is that soc clk driver should suspend > after every other suspend callback which is using clocks... It would > be nice to explain this particular scenario in commit msg. > > However both dw-mmc and clk will be now in suspend noirq phase so do > you have any guarantees that dw-mmc will be suspended after clk? Frankly speaking this works now, because the devices are populated in the order of their presence in device-tree. When the order would be reverse, dw-mmc driver will defer probe until clocks are registered. This would be enough to ensure proper suspend/resume order, because all deferred devices are moved to the end of dpm_list (the list of device used for system suspend/resume calls), see deferred_probe_work_func() and comments there. This is however still not fully resolved problem that has to be addressed one day. Andrzej Hajda will have a speech on this topic at Open Source Summit: https://osseu18.sched.com/event/FxYd/deferred-problem-issues-with-complex-dependencies-between-devices-in-linux-kernel-andrzej-hajda-samsung Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
