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
