Messages in this thread | | | Date | Mon, 4 Jan 2021 14:05:12 +0200 | From | Abel Vesa <> | Subject | Re: [PATCH] Revert "clk: imx: fix composite peripheral flags" |
| |
On 20-12-31 17:33:40, Fabio Estevam wrote: > Hi Martin, > > On Thu, Dec 31, 2020 at 11:22 AM Martin Kepplinger > <martin.kepplinger@puri.sm> wrote: > > > > This reverts commit 936c383673b9e3007432f17140ac62de53d87db9. > > > > It breaks clock reparenting via devfreq on the imx8mq used in the > > Librem 5 phone. When switching dram frequency (which worked before) > > the system now hangs after this where the dram_apb clock cannot be > > set: > > > > [ 129.391755] imx8m-ddrc-devfreq 3d400000.memory-controller: failed to > > set dram_apb parent: -16 > > [ 129.391959] imx8m-ddrc-devfreq 3d400000.memory-controller: ddrc > > failed freq switch to 25000000 from 800000000: error -16. now at 25000000 > > [ 129.406133] imx8m-ddrc-devfreq 3d400000.memory-controller: failed to > > update frequency from PM QoS (-16) > > I am wondering whether IMX8MQ_CLK_DRAM_ALT should also be marked as > CLK_IS_CRITICAL. >
Hmm, the way the DRAM clocks are right registered right now is a real mess. The DRAM clocks on i.MX8M are changed in TF-A, but the kernel still needs to register them to keep track of the clock tree.
Martin, I already have a patchset waiting to be shipped which doesn't only fix the 8MQ, but all the 8M platforms. Unfortunately, I haven't had the time to work on that in the last couple of weeks but I intend to switch back to it soon.
Fabio, marking the DRAM clocks as critical will not allow the set_parent to be done, as CLK_IS_CRITICAL flag and set_parent do not go together. As of now the devfreq tries to reparent to be consistent with TF-A configuration.
My approach here was to make the DRAM clocks read-only. This means adding some stuff in the clock core subsystem too.
> Could you please try the following change without the revert? > > --- a/drivers/clk/imx/clk-imx8mq.c > +++ b/drivers/clk/imx/clk-imx8mq.c > @@ -458,7 +458,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev) > * Mark with GET_RATE_NOCACHE to always read div value from hardware > */ > hws[IMX8MQ_CLK_DRAM_CORE] = > imx_clk_hw_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, > imx8mq_dram_core_sels, ARRAY_SIZE(imx8mq_dram_core_sels), > CLK_IS_CRITICAL); > - hws[IMX8MQ_CLK_DRAM_ALT] = > __imx8m_clk_hw_composite("dram_alt", imx8mq_dram_alt_sels, base + > 0xa000, CLK_GET_RATE_NOCACHE); > + hws[IMX8MQ_CLK_DRAM_ALT] = > __imx8m_clk_hw_composite("dram_alt", imx8mq_dram_alt_sels, base + > 0xa000, CLK_IS_CRITICAL | CLK_GET_RATE_NOCACHE); > hws[IMX8MQ_CLK_DRAM_APB] = > __imx8m_clk_hw_composite("dram_apb", imx8mq_dram_apb_sels, base + > 0xa080, CLK_IS_CRITICAL | CLK_GET_RATE_NOCACHE); > > Thanks
| |