GPMI iMX6ull timeout on DMA

[Miquel Raynal]

Hello,

S.Riedmueller at phytec.de wrote on Fri, 8 Oct 2021 12:08:01 +0000:

> Hi Christian,
> 
> On Fri, 2021-10-08 at 11:55 +0200, Christian Eggers wrote:
> > + set PHYTEC developers on (B)CC
> > 
> > On Monday, 4 October 2021, 18:06:20 CEST, Han Xu wrote:  
> > > Could you please try to add clock dis/enable when setting clock rate, in
> > > case
> > > clock glitches.
> > > 
> > > clk_disable_unprepare(r->clock[0]);
> > > clk_set_rate(r->clock[0], hw->clk_rate);
> > > clk_prepare_enable(r->clock[0]);
> > >   
> > 
> > With this change, we made over 2000 successful reboots without any GPMI DMA
> > timeout problems!
> > 
> > From PHYTEC (our BSP supplier), I got some possible background for this
> > problem. For older revisions of IMX6DQ there was an errata (ERR007117, [1])
> > in the ROM bootloader which triggers a similar / the same behavior:
> >   
> > > For raw NAND boot, ROM switches the source of enfc_clk_root from PLL2_PFD2
> > > to PLL3. The root clock is required to be gated before switching the
> > > source
> > > clock. If the root clock is not gated, clock glitches might be passed to
> > > the divider that follows the clock mux, and the divider might behave
> > > unpredictably.
> > > ...
> > > This problem can also occur elsewhere in application code if the root
> > > clock
> > > is not properly gated when the clock configuration is changed.  
> > 
> > In my case (Linux boot on i.MX6ULL), I recognized that on the 3rd call of
> > gpmi_nfc_apply_timings(), the rate of r->clock[0] is changed 
> > 
> > - from: 22 MHz (CS2CDR::ENFC_CLK_PRED=6 and CS2CDR::ENFC_CLK_PODF=3)
> > - to:  100 MHz (CS2CDR::ENFC_CLK_PRED=4 and CS2CDR::ENFC_CLK_PODF=1)
> > 
> > The proposal from from Han Xu  
> > > clk_disable_unprepare(r->clock[0]);
> > > clk_set_rate(r->clock[0], hw->clk_rate);
> > > clk_prepare_enable(r->clock[0]);  
> > disables only CCGR4::CG14 [RAWNAND_U_GPMI_BCH_INPUT_GPMI_IO_CLK_ENABLE]
> > during
> > the rate change. While this works very fine on my system, this probably
> > doesn't
> > fulfill the requirements in the errata description:
> >   
> > > For other occurrences in application code, the following procedure should
> > > be
> > > followed to change the clock configuration for the enfc_clk_root:
> > > 1) Gate (disable) the GPMI/BCH clocks in register CCM_CCGR4.
> > > 2) Gate (disable) the enfc_clk_root before changing the enfc_clk_root
> > > source
> > >    or dividers by clearing CCM_CCGR2[CG7] to 2’b00. This disables the
> > >    iomux_ipt_clk_io_clk.
> > > 3) Configure CCM_CS2CDR for the new clock source configuration.
> > > 4) Enable enfc_clk_root by setting CCM_CCGR2[CG7] to 2’b11. This enables
> > > the
> > >    iomux_ipt_clk_io_clk.
> > > 5) Enable the GPMI/BCH clocks in register CCM_CCGR4  
> > 
> > I got another solution from PHYTEC ([2], not lengthy tested yet), which
> > disables all GPMI/BCH clocks on CCGR4 (verified with a JTAG debugger):
> > - CCGR4::CG15  [RAWNAND_U_GPMI_INPUT_APB_CLK_ENABLE]
> > - CCGR4::CG14  [RAWNAND_U_GPMI_BCH_INPUT_GPMI_IO_CLK_ENABLE]
> > - CCGR4::CG13  [RAWNAND_U_GPMI_BCH_INPUT_BCH_CLK_ENABLE]
> > - CCGR4::CG12  [RAWNAND_U_BCH_INPUT_APB_CLK_ENABLE]
> > - CCGR4::CG6   [PL301_MX6QPER1_BCHCLK_ENABLE]
> > 
> > CCM_CCGR2[CG7] is used for IOMUX_IPT_CLK_IO_ENABLE on i.MX6ULL, so step 2.
> > seems not to apply here. Actually I don't know how to gate ENFC_CLK_ROOT.
> > 
> > I will cyclic test the solution from PHYTEC over the weekend.
> > 
> > @Han Xu: Should I prefer the solution from PHYTEC?
> > @Stefan Riedmueller: Are you willing to commit this upstream?  
> 
> Yes sure, I can prepare a patch beginning of next week.
> BTW, we have seen these DMA timeout issues on the i.MX6 SOCs as well. So this
> fix is not only for the i.MX 6ULL.

Could it be possible to quantify the extra time spent in
disabling/re-enabling clocks just for the record? So far this driver
only supports a single chip and thus frequency changes do not happen
frequently (a couple times at boot) but if someday we introduce support
for several chips it might become very impacting. 

Thanks,
Miquèl