On 21.03.2023 14:56, Georgi Djakov wrote: > Hi Konrad, > > Thank you for working on this and sorry about jumping a bit late into > the discussion. > > On 8.03.23 23:40, Konrad Dybcio wrote: >> Some (but not all) providers (or their specific nodes) require >> specific clocks to be turned on before they can be accessed. Failure >> to ensure that results in a seemingly random system crash (which >> would usually happen at boot with the interconnect driver built-in), >> resulting in the platform not booting up properly. > > These "interface" clocks seem to be used only to program QoS for the > respective ip block (eg ufs). So if we don't program QoS, there should > be no crashes, right? Correct. > > I believe that in downstream they defer setting QoS until the first > non-zero bandwidth request because of drivers that probe asynchronously > or there is some firmware booting involved (IPA maybe). Hmm.. that would make sense. > And bad stuff > might happen if we touch the clock while the firmware is still booting. > So setting the QoS on the first non-zero bandwidth request might not be > a bad idea. Sounds like a plan, I don't think setting QoS parameters on buses that are not in use does anything (citation needed). Such nodes should probably be also excluded from sync_state > by implementing get_bw() to return 0 bandwidth. I agree, setting the QoS parameters in sync_state (so, without their dependencies fully met) sounds like a recipe for disaster, while getting rid of QoS in sync_state at all would leave the nodes that are not explicitly referenced in the device tree unconfigured. Konrad > > BR, > Georgi > >> >> Limit the number of bus_clocks to 2 (which is the maximum that SMD >> RPM interconnect supports anyway) and handle non-scaling clocks >> separately. Update MSM8996 and SDM660 drivers to make sure they do >> not regress with this change. >> >> This unfortunately has to be done in one patch to prevent either >> compile errors or broken bisect. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >> --- >> drivers/interconnect/qcom/icc-rpm.c | 52 ++++++++++++++++++++++++++++++------- >> drivers/interconnect/qcom/icc-rpm.h | 14 ++++++++-- >> drivers/interconnect/qcom/msm8996.c | 22 +++++++--------- >> drivers/interconnect/qcom/sdm660.c | 16 +++++------- >> 4 files changed, 70 insertions(+), 34 deletions(-) >> > [..] >
