On Mon, Aug 17, 2020 at 09:46:08AM -0600, Jeffrey Hugo wrote:
> > > So essentially, when the clk framework goes through late init, and
> > > decides to turn off clocks that are not being used, it will also turn
> > > off these clocks?
> > >
> >
> > With this patch: yes.
> >
> > > I think this is going to break other targets where other subsystems
> > > happen to rely on these sorts of votes from Linux inorder to run/boot
> > > (not saying it's a good thing, just that is how it is and since we
> > > can't change the FW on those....).
> > >
> >
> > As far as I can tell the behavior implemented in this patch (= force
> > clocks on during boot but disable them when unused) is the same on that
> > is used on the downstream kernel. Most FW is probably written with the
> > downstream kernel in mind, so I don't think this is going to cause trouble.
>
> Based on my experience with 8998, I disagree. I would need to dig up
> the history for specifics.
>
I don't know anything about 8998, so it's possible.
My statement was based on a quick look at the downstream code:
For some reason there is an entirely separate MSM clock framework
downstream:
1. During msm_clock_register() [1] it calls __handoff_clk()
for all the clocks.
2. __handoff_clk() [2] calls clk->ops->handoff(clk) and if that returns
success (HANDOFF_ENABLED_CLK) it adds the clock to a "handoff_list".
-> rpm_clk_handoff() [3] forces the clock on similar to mainline.
3. In a late init call (clock_late_init()) [4] it iterates over
"handoff_list" and reduces the prepare_count again and eventually
disables the clock.
In this patch I implement something equivalent to (3).
[1]: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/clk/msm/clock.c?h=LA.UM.7.2.r2-06200-8x98.0#n985
[2]: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/clk/msm/clock.c?h=LA.UM.7.2.r2-06200-8x98.0#n873
[3]: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/clk/msm/clock-rpm.c?h=LA.UM.7.2.r2-06200-8x98.0#n263
[4]: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/clk/msm/clock.c?h=LA.UM.7.2.r2-06200-8x98.0#n1351
> >
> > > Also, out of curiosity, how are you validating that BB_CLK2 is
> > > actually off after this change?
> > >
> >
> > Since BB_CLK1/2 and RF_CLK1/2 are part of the PMIC (at least on MSM8916)
> > I used the regmap debugfs interface to read the clock registers
> > through SPMI from Linux.
> >
> > From the "PM8916 Hardware Register Description" [1] I got the registers
> > mentioned in the table, e.g. for BB_CLK2:
> >
> > 0x5208: BB_CLK2_STATUS1
> > BIT(7): CLK_OK (Indicates Hardware or Software enable and
> > includes warmup delay)
> > 0x0: BBCLK_OFF
> > 0x1: BBCLK_ON
> >
> > I read the registers from /sys/kernel/debug/regmap/0-00/registers:
> >
> > Without this patch:
> > 5108: 80
> > 5208: 80
> > 5408: 80
> > 5508: 80
> >
> > With this patch (and with clk-smd-rpm entirely disabled):
> > 5108: 80
> > 5208: 00
> > 5408: 00
> > 5508: 00
> >
> > Stephan
> >
> > [1]: https://developer.qualcomm.com/download/sd410/pm8916-hardware-register-description.pdf
>
> Hmm, 8916 is probably old enough where you can actually do that. For
> the modern SoCs, you'll have to go through jtag to get an accurate
> view of the clocks.
I guess I was lucky then :)
Stephan
