Re: [PATCH] clk: qcom: smd: Disable unused clocks

[Stephan Gerhold <stephan@gerhold.net>]

On Mon, Aug 17, 2020 at 08:52:46AM -0600, Jeffrey Hugo wrote:
> > Overall I'm not entirely sure why we need to force all these clocks
> > on at all... But the downstream driver also seems to do it and the RPM
> > interface is barely documented, so I didn't feel comfortable changing it...
> 
> 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.

The only situation this patch could break something is if we forgot to
manage the clocks for one of the devices in mainline
(and implicitly relied on clk-smd-rpm to keep them always-on).

For example, one situation I checked is for WCNSS on MSM8916.
It seems to require RF_CLK2 to boot. However, this is already handled in
qcom_wcnss_iris.c where the clock is forced on until WCNSS is ready.

> I think this needs to be validated on every single qcom platform using
> this driver.
> 
> 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