Hi Geert, Thanks for your feedback. On 2019-05-08 11:03:05 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > On Mon, Apr 15, 2019 at 1:10 PM Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote: > > On Thu, Apr 11, 2019 at 12:22:40AM +0200, Niklas Söderlund wrote: > > > Both the Renesas and Uniphier implementations perform actions which > > > affect runtime PM before calling into the core tmio_mmc_host_probe() > > Do you know which pm_runtime_*() calls were done too early? > I guess they returned an error, which is not checked? The ones in tmio_mmc_host_probe() which is removed in this patch where called before PM was enabled. > > I checked the various pm_runtime_get*() calls, but none of them failed, > while they typically return -EACCES when called too early. > > > > which enabled runtime PM. Move pm_runtime_enable() from the core and > > > tmio_mmc_host_probe() into each drivers probe() so it can be called > > > before any clocks or other resources are switched on. > > > > > > Reported-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > > > --- > > > > Thanks for keeping at this. Setting up the APE6 board for further tests > > was painful, I understood that. > > > > Since you lost the cover-letter from the last series, I think it should > > be mentioned that this fixes a clock imbalance problem (at least on > > Gen3). > > > > For the APE6 tests, we need to wait until Geert comes back. I surely > > would like his input. And Yamada-san's, too, to make sure his platform > > also benefits. > > Thanks, but I still see a clock imbalances in /sys/kernel/debug/clk/clk_summary > when comparing before/after s2ram. > > On ape6evm: > > - mmcif0 2 2 0 100000000 0 0 50000 > + mmcif0 1 1 0 100000000 0 0 50000 This is unrelated to this patch, this clock is handled by the sh_mmcif driver. I get the same diff of the mmcif0 clock with a suspend cycle even if i do not include the renesas_sdhi_* drivers in the system. I had a quick look at the issue and it's related to that the MCC core do not call MMC_POWER_UP after suspend while it do call it during boot. Why it does so I'm not sure. Also if I mock convert sh_mmcif to require PM the imbalance is gone which perplexes me a bit and wonder if I converted it wrong somehow. > > On r8a77965/salvator-xs: > > - s0d3 1 2 0 266240000 0 0 50000 > + s0d3 2 2 0 266240000 0 0 50000 > > - sys-dmac0 0 1 0 266240000 0 0 50000 > + sys-dmac0 1 1 0 266240000 0 0 50000 Even these are unrelated to this patch. If I test without renesas_sdhi_* driver in the system I get the same clock differences, in fact I get one more for sys-dmac1 (both with and without the shdi drivers). - s0d3 2 6 0 266240000 0 0 50000 + s0d3 4 6 0 266240000 0 0 50000 - sys-dmac0 0 1 0 266240000 0 0 50000 + sys-dmac0 1 1 0 266240000 0 0 50000 - sys-dmac1 0 1 0 266240000 0 0 50000 + sys-dmac1 1 1 0 266240000 0 0 50000 I have not investigate this further as I wish to make sens of this patch first ;-) Would you agree that with this information we should move forward with this patch as it solves the issue for the sdhi clocks on all effected SoCs ? -- Regards, Niklas Söderlund
