[PATCH v3 00/10] clk: implement clock rate protection mechanism
Jerome Brunet
jbrunet at baylibre.com
Wed Jul 26 10:05:47 PDT 2017
On Tue, 2017-07-11 at 18:16 -0700, Stephen Boyd wrote:
> On 06/21, Jerome Brunet wrote:
> >
> > I managed to have a run on kci - based on v4.12-rc6:
> > https://kernelci.org/boot/all/job/khilman/branch/to-build/kernel/v4.12-rc6-1
> > 0-ge
> > a373ddef830/
> >
> > There was no build regression but kci did find one boot regression on qcom
> > platforms:
> > * qcom-apq8064-cm-qs600
> > * qcom-apq8064-ifc6410
> >
> > it seems the problem is coming from the clock used by the mmc driver
> > (drivers/mmc/host/mmci.c)
> >
> > the driver does following sequence:
> > * get_clk
> > * prepare_enable
> > * get_rate
> > * set_rate
> > * ...
> >
> > with clock SDCx_clk (qcom_apq8064.dtsi:1037). This clock has
> > CLK_SET_RATE_PARENT
> > flag so it will transmit the request to its parent.
> > The parent of this clock is SDCx_src which has the CLK_SET_RATE_GATE flag.
> >
> > This can't work with sequence above. The flag was clearly bypassed before,
> > so I
> > think it would be best to remove CLK_SET_RATE_GATE from the SDCx_src clocks
> >
> > Stephen, would you agree ?
> >
> > There was an issue with the chip board as well but this particular board has
> > not
> > been very reliable in this lab lately. It has failed on several other job.
> >
>
> Yes the flag was never working before and we've been ignoring the
> subtle problems that it causes to change the rate while the clk
> is enabled. On these older SoCs, all the clks marked with
> CLK_SET_RATE_GATE need to implement something to forcibly disable
> all child clks across the rate change. That is the hardware
> recommended procedure.
Seems that HW has been doing fine w/o recommended procedure so far ;)
>
> So something like
>
> get_clk
> prepare_enable
> get_rate
> set_rate
> call ->set_rate() op
> forcibly disable children of rcgs
> actually change the rate of rcg
> forcibly enable all children that were disabled
>
> and have that happen under one big spinlock local to the qcom
> driver I suppose so that enable/disable can't leak into the rate
> change. Fun! I'll make an attempt at implementing the right
> solution tomorrow/tonight. Of course, we can remove the flag from
> qcom drivers if it's blocking this series.
>
Yeah, it is probably blocking the series.
In the following mail, I've only replied where I wanted to discuss a bit more.
For everything else, please consider the comments agreed and the modifications
done as requested.
More information about the linux-amlogic
mailing list