| Message ID | 20230228-topic-qos-v7-6-815606092fff@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | The great interconnecification fixation | expand |
On 08/03/2023 21: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. Can you give an example of which clocks on which SoC's ? Is the intention of this patch to subsequently go through *.dts *.dtsi and start to remove assigned-clocks ? Are we saying that currently there ought to be assigned-clocks for some of these NoC declarations ? --- bod
On 10.03.2023 15:21, Bryan O'Donoghue wrote: > On 08/03/2023 21: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. > > Can you give an example of which clocks on which SoC's ? See for example 67fb53745e0b This was a clock documented downstream under the node-qos clocks here: https://github.com/sonyxperiadev/kernel/blob/aosp/LA.UM.5.7.r1/arch/arm/boot/dts/qcom/msm8996-bus.dtsi#L102-L109 but there are occasions where such clocks are undocumented and downstream skips them because it relies on them being on by miracle, such as the case of MASTER_IPA and the IPA rpmcc clock on msm8998. Downstream has no sync_state, so they would only set the QoS registers when the relevant hardware was online, so the clocks were on already. > > Is the intention of this patch to subsequently go through *.dts *.dtsi and start to remove assigned-clocks ? > > Are we saying that currently there ought to be assigned-clocks for some of these NoC declarations ? Not really, assigned-clocks are used for static ratesetting, see for example dwc3 nodes where we need it to be fast enough for HS/SS operation at all times (though that should have prooobably been handled in the driver but it's a separate topic), I don't think any of them were used to combat what this commit tries to. Konrad > > --- > bod
On 11.03.2023 13:11, Bryan O'Donoghue wrote: > On 11/03/2023 00:54, Konrad Dybcio wrote: >>> - Update the commit log and potentially the structure comments >> I'm probably just very biased because I authored these commits, but I can't >> see which part is not clear.. Could I (and this is not passive-aggressive or >> anything) ask for a pointer there? >> > > I mean to say "non scaling clocks" isn't an indicator IMO of the fact that these are QoS node specific clocks. > > Right now the interconnect model is predicated on bus and bus_a but, you've found that on some SoCs we have node-specific clocks too. > > :g/non\ scaling/s//non-scaling\ node-specific/g > > would do or "QoS node-specific" the fact the clocks don't scale is incidental the dependency though is that IMO at least these are additional node-specific clocks we need to enable. Okay that makes sense. Thanks. Konrad > > --- > bod
On 11/03/2023 14:01, Dmitry Baryshkov wrote: >> >> 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. > > Can we determine somehow if the intf clocks are required for the whole > NoC or just for a single node? I don't think that clocks like a0noc_ufs > are requiered to be up for the whole aggre_noc. Instead they probably > have to be enabled only when UFS makes use of the NoC (in other words > when is has voted for the bandwidth). Its probably worthwhile experimenting to see if the *ufs*_clk can/should be added to the UFS device list of clocks. I hadn't realised we were talking about enabling the intf clocks always but, actually that is what we are talking about isn't it ? It seems pretty unlikely that other devices would depend on clocks named *ufs* OTOH "clk-aggre2-noc-clk-no-rate" may be used across different nodes, seems like a pretty generic name, though still suspicious it is bundled with UFS, likely it is only required for UFS ?!? Konrad can you try: 1. Moving the intf_clks/QoS clocks be contained to UFS alone ? 2. If that doesn't work just the *ufs* clocks be put there with 2a. "clk-aggre2-noc-clk-no-rate" alone added to the intf_clk list It seems wrong to be enabling ufs related NoC clocks unless we are actually switching on UFS. --- bod
On 11/03/2023 14:35, Dmitry Baryshkov wrote: >> Its probably worthwhile experimenting to see if the*ufs*_clk can/should >> be added to the UFS device list of clocks. > While we were doing this for some of the clocks (PCIe and USB, if I'm > not mistaken), I think that generally this is not fully correct. In my > opinion it should be in the interconnect driver, who turns > corresponding clocks on and off. These clocks correspond to the SoC > topology, rather than the end-device. > True enough, they are interconnect clocks. The question is how to only turn them on when the device that depends on them wants them. --- bod
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? 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). 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. Such nodes should probably be also excluded from sync_state by implementing get_bw() to return 0 bandwidth. 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@linaro.org> > --- > 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(-) > [..]
Hi, On 11.03.23 17:26, Dmitry Baryshkov wrote: > On 11/03/2023 16:38, Bryan O'Donoghue wrote: >> On 11/03/2023 14:35, Dmitry Baryshkov wrote: >>>> Its probably worthwhile experimenting to see if the*ufs*_clk can/should >>>> be added to the UFS device list of clocks. >>> While we were doing this for some of the clocks (PCIe and USB, if I'm >>> not mistaken), I think that generally this is not fully correct. In my >>> opinion it should be in the interconnect driver, who turns >>> corresponding clocks on and off. These clocks correspond to the SoC >>> topology, rather than the end-device. >>> >> >> True enough, they are interconnect clocks. >> >> The question is how to only turn them on when the device that depends on them wants them. > > I think we can turn them on an off from qcom_icc_set(). Each node can list required clocks. > Yes, this is a bit weird, but looks like these are the interface clocks required for programming the qos box of the respective peripheral and nothing else. Maybe we can even configure QoS just once (eg. on the first bandwidth request) and not every time we call qcom_icc_set(). BR, Georgi
On 21.03.2023 14:58, Georgi Djakov wrote: > Hi, > > On 11.03.23 17:26, Dmitry Baryshkov wrote: >> On 11/03/2023 16:38, Bryan O'Donoghue wrote: >>> On 11/03/2023 14:35, Dmitry Baryshkov wrote: >>>>> Its probably worthwhile experimenting to see if the*ufs*_clk can/should >>>>> be added to the UFS device list of clocks. >>>> While we were doing this for some of the clocks (PCIe and USB, if I'm >>>> not mistaken), I think that generally this is not fully correct. In my >>>> opinion it should be in the interconnect driver, who turns >>>> corresponding clocks on and off. These clocks correspond to the SoC >>>> topology, rather than the end-device. >>>> >>> >>> True enough, they are interconnect clocks. >>> >>> The question is how to only turn them on when the device that depends on them wants them. >> >> I think we can turn them on an off from qcom_icc_set(). Each node can list required clocks. >> > > Yes, this is a bit weird, but looks like these are the interface clocks > required for programming the qos box of the respective peripheral and > nothing else. Maybe we can even configure QoS just once (eg. on the first > bandwidth request) and not every time we call qcom_icc_set(). Would that persist a full bus reset - if we e.g. shut down MMNoC after the display stack is turned off in preparation for a power collapse, would we have to reprogram it? Another thing is, do we know "how persistent" the QoS settings are? What could reset them? Would a bandwidth request for a node that belongs to the same path do so? Konrad > > BR, > Georgi
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@linaro.org> >> --- >> 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(-) >> > [..] >
On 21.03.23 16:11, Konrad Dybcio wrote: > > > On 21.03.2023 14:58, Georgi Djakov wrote: >> Hi, >> >> On 11.03.23 17:26, Dmitry Baryshkov wrote: >>> On 11/03/2023 16:38, Bryan O'Donoghue wrote: >>>> On 11/03/2023 14:35, Dmitry Baryshkov wrote: >>>>>> Its probably worthwhile experimenting to see if the*ufs*_clk can/should >>>>>> be added to the UFS device list of clocks. >>>>> While we were doing this for some of the clocks (PCIe and USB, if I'm >>>>> not mistaken), I think that generally this is not fully correct. In my >>>>> opinion it should be in the interconnect driver, who turns >>>>> corresponding clocks on and off. These clocks correspond to the SoC >>>>> topology, rather than the end-device. >>>>> >>>> >>>> True enough, they are interconnect clocks. >>>> >>>> The question is how to only turn them on when the device that depends on them wants them. >>> >>> I think we can turn them on an off from qcom_icc_set(). Each node can list required clocks. >>> >> >> Yes, this is a bit weird, but looks like these are the interface clocks >> required for programming the qos box of the respective peripheral and >> nothing else. Maybe we can even configure QoS just once (eg. on the first >> bandwidth request) and not every time we call qcom_icc_set(). > Would that persist a full bus reset - if we e.g. shut down MMNoC > after the display stack is turned off in preparation for a power > collapse, would we have to reprogram it? > > Another thing is, do we know "how persistent" the QoS settings are? > What could reset them? Would a bandwidth request for a node that > belongs to the same path do so? That's a good question. From what i recall, i expect them to persist until you reset the board. Probably we can verify it with an experiment by reading them back, but let me check if i can find some info. Thanks, Georgi
On 21.03.23 16:38, Georgi Djakov wrote: > On 21.03.23 16:11, Konrad Dybcio wrote: >> >> >> On 21.03.2023 14:58, Georgi Djakov wrote: >>> Hi, >>> >>> On 11.03.23 17:26, Dmitry Baryshkov wrote: >>>> On 11/03/2023 16:38, Bryan O'Donoghue wrote: >>>>> On 11/03/2023 14:35, Dmitry Baryshkov wrote: >>>>>>> Its probably worthwhile experimenting to see if the*ufs*_clk can/should >>>>>>> be added to the UFS device list of clocks. >>>>>> While we were doing this for some of the clocks (PCIe and USB, if I'm >>>>>> not mistaken), I think that generally this is not fully correct. In my >>>>>> opinion it should be in the interconnect driver, who turns >>>>>> corresponding clocks on and off. These clocks correspond to the SoC >>>>>> topology, rather than the end-device. >>>>>> >>>>> >>>>> True enough, they are interconnect clocks. >>>>> >>>>> The question is how to only turn them on when the device that depends on them wants them. >>>> >>>> I think we can turn them on an off from qcom_icc_set(). Each node can list required clocks. >>>> >>> >>> Yes, this is a bit weird, but looks like these are the interface clocks >>> required for programming the qos box of the respective peripheral and >>> nothing else. Maybe we can even configure QoS just once (eg. on the first >>> bandwidth request) and not every time we call qcom_icc_set(). >> Would that persist a full bus reset - if we e.g. shut down MMNoC >> after the display stack is turned off in preparation for a power >> collapse, would we have to reprogram it? >> >> Another thing is, do we know "how persistent" the QoS settings are? >> What could reset them? Would a bandwidth request for a node that >> belongs to the same path do so? > > That's a good question. From what i recall, i expect them to persist until > you reset the board. Probably we can verify it with an experiment by reading > them back, but let me check if i can find some info. > This seems to be hardware specific and there is no general answer. It depends on where the reset line for the NIU comes from. It could be from the primary chip reset in most cases, but it could be also within the power domain of the associated core. Thanks, Georgi
diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c index b52f788d8f3d..ca932ed720fb 100644 --- a/drivers/interconnect/qcom/icc-rpm.c +++ b/drivers/interconnect/qcom/icc-rpm.c @@ -369,6 +369,17 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst) qcom_icc_bus_aggregate(provider, agg_avg, agg_peak, &max_agg_avg); + /* If we're powering on the bus, ensure the necessary clocks are on */ + if (unlikely(!qp->is_on)) { + if (agg_peak[0] || agg_peak[1] || max_agg_avg) { + /* If this fails, bus accesses will crash the platform! */ + ret = clk_bulk_prepare_enable(qp->num_intf_clks, qp->intf_clks); + if (ret) + return ret; + qp->is_on = true; + } + } + sum_bw = icc_units_to_bps(max_agg_avg); ret = __qcom_icc_set(src, src_qn, sum_bw); @@ -409,6 +420,14 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst) qp->bus_clk_rate[i] = rate; } + /* Turn off the interface clocks if the bus was shut down so as not to leak power */ + if (!qp->bus_clk_rate[0] && !qp->bus_clk_rate[1]) { + if (!agg_peak[0] && !agg_peak[1] && !max_agg_avg) { + clk_bulk_disable_unprepare(qp->num_intf_clks, qp->intf_clks); + qp->is_on = false; + } + } + return 0; } @@ -441,21 +460,20 @@ int qnoc_probe(struct platform_device *pdev) qnodes = desc->nodes; num_nodes = desc->num_nodes; - if (desc->num_bus_clocks) { - cds = desc->bus_clocks; - cd_num = desc->num_bus_clocks; + if (desc->num_intf_clocks) { + cds = desc->intf_clocks; + cd_num = desc->num_intf_clocks; } else { - cds = bus_clocks; - cd_num = ARRAY_SIZE(bus_clocks); + /* 0 intf clocks is perfectly fine */ + cd_num = 0; } - qp = devm_kzalloc(dev, struct_size(qp, bus_clks, cd_num), GFP_KERNEL); + qp = devm_kzalloc(dev, sizeof(*qp), GFP_KERNEL); if (!qp) return -ENOMEM; - qp->bus_clk_rate = devm_kcalloc(dev, cd_num, sizeof(*qp->bus_clk_rate), - GFP_KERNEL); - if (!qp->bus_clk_rate) + qp->intf_clks = devm_kzalloc(dev, sizeof(qp->intf_clks), GFP_KERNEL); + if (!qp->intf_clks) return -ENOMEM; data = devm_kzalloc(dev, struct_size(data, nodes, num_nodes), @@ -463,6 +481,18 @@ int qnoc_probe(struct platform_device *pdev) if (!data) return -ENOMEM; + qp->num_intf_clks = cd_num; + for (i = 0; i < cd_num; i++) + qp->intf_clks[i].id = cds[i]; + + if (desc->num_bus_clocks) { + cds = desc->bus_clocks; + cd_num = desc->num_bus_clocks; + } else { + cds = bus_clocks; + cd_num = ARRAY_SIZE(bus_clocks); + } + for (i = 0; i < cd_num; i++) qp->bus_clks[i].id = cds[i]; qp->num_bus_clks = cd_num; @@ -503,6 +533,10 @@ int qnoc_probe(struct platform_device *pdev) if (ret) return ret; + ret = devm_clk_bulk_get(dev, qp->num_intf_clks, qp->intf_clks); + if (ret) + return ret; + if (desc->has_bus_pd) { ret = dev_pm_domain_attach(dev, true); goto err_disable_clks; diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h index d4401f35f6d2..a4ef45b4a9e0 100644 --- a/drivers/interconnect/qcom/icc-rpm.h +++ b/drivers/interconnect/qcom/icc-rpm.h @@ -20,24 +20,32 @@ enum qcom_icc_type { QCOM_ICC_QNOC, }; +#define NUM_BUS_CLKS 2 + /** * struct qcom_icc_provider - Qualcomm specific interconnect provider * @provider: generic interconnect provider * @num_bus_clks: the total number of bus_clks clk_bulk_data entries + * @num_intf_clks: the total number of intf_clks clk_bulk_data entries * @type: the ICC provider type * @regmap: regmap for QoS registers read/write access * @qos_offset: offset to QoS registers * @bus_clk_rate: bus clock rate in Hz * @bus_clks: the clk_bulk_data table of bus clocks + * @intf_clks: a clk_bulk_data array of interface clocks + * @is_on: whether the bus is powered on */ struct qcom_icc_provider { struct icc_provider provider; int num_bus_clks; + int num_intf_clks; enum qcom_icc_type type; struct regmap *regmap; unsigned int qos_offset; - u64 *bus_clk_rate; - struct clk_bulk_data bus_clks[]; + u64 bus_clk_rate[NUM_BUS_CLKS]; + struct clk_bulk_data bus_clks[NUM_BUS_CLKS]; + struct clk_bulk_data *intf_clks; + bool is_on; }; /** @@ -93,6 +101,8 @@ struct qcom_icc_desc { size_t num_nodes; const char * const *bus_clocks; size_t num_bus_clocks; + const char * const *intf_clocks; + size_t num_intf_clocks; bool has_bus_pd; enum qcom_icc_type type; const struct regmap_config *regmap_cfg; diff --git a/drivers/interconnect/qcom/msm8996.c b/drivers/interconnect/qcom/msm8996.c index 69fc50a6fa5c..1a5e0ad36cc4 100644 --- a/drivers/interconnect/qcom/msm8996.c +++ b/drivers/interconnect/qcom/msm8996.c @@ -21,21 +21,17 @@ #include "smd-rpm.h" #include "msm8996.h" -static const char * const bus_mm_clocks[] = { - "bus", - "bus_a", +static const char * const mm_intf_clocks[] = { "iface" }; -static const char * const bus_a0noc_clocks[] = { +static const char * const a0noc_intf_clocks[] = { "aggre0_snoc_axi", "aggre0_cnoc_ahb", "aggre0_noc_mpu_cfg" }; -static const char * const bus_a2noc_clocks[] = { - "bus", - "bus_a", +static const char * const a2noc_intf_clocks[] = { "aggre2_ufs_axi", "ufs_axi" }; @@ -1821,8 +1817,8 @@ static const struct qcom_icc_desc msm8996_a0noc = { .type = QCOM_ICC_NOC, .nodes = a0noc_nodes, .num_nodes = ARRAY_SIZE(a0noc_nodes), - .bus_clocks = bus_a0noc_clocks, - .num_bus_clocks = ARRAY_SIZE(bus_a0noc_clocks), + .intf_clocks = a0noc_intf_clocks, + .num_intf_clocks = ARRAY_SIZE(a0noc_intf_clocks), .has_bus_pd = true, .regmap_cfg = &msm8996_a0noc_regmap_config }; @@ -1866,8 +1862,8 @@ static const struct qcom_icc_desc msm8996_a2noc = { .type = QCOM_ICC_NOC, .nodes = a2noc_nodes, .num_nodes = ARRAY_SIZE(a2noc_nodes), - .bus_clocks = bus_a2noc_clocks, - .num_bus_clocks = ARRAY_SIZE(bus_a2noc_clocks), + .intf_clocks = a2noc_intf_clocks, + .num_intf_clocks = ARRAY_SIZE(a2noc_intf_clocks), .regmap_cfg = &msm8996_a2noc_regmap_config }; @@ -2005,8 +2001,8 @@ static const struct qcom_icc_desc msm8996_mnoc = { .type = QCOM_ICC_NOC, .nodes = mnoc_nodes, .num_nodes = ARRAY_SIZE(mnoc_nodes), - .bus_clocks = bus_mm_clocks, - .num_bus_clocks = ARRAY_SIZE(bus_mm_clocks), + .intf_clocks = mm_intf_clocks, + .num_intf_clocks = ARRAY_SIZE(mm_intf_clocks), .regmap_cfg = &msm8996_mnoc_regmap_config }; diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c index a22ba821efbf..0e8a96f4ce90 100644 --- a/drivers/interconnect/qcom/sdm660.c +++ b/drivers/interconnect/qcom/sdm660.c @@ -127,15 +127,11 @@ enum { SDM660_SNOC, }; -static const char * const bus_mm_clocks[] = { - "bus", - "bus_a", +static const char * const mm_intf_clocks[] = { "iface", }; -static const char * const bus_a2noc_clocks[] = { - "bus", - "bus_a", +static const char * const a2noc_intf_clocks[] = { "ipa", "ufs_axi", "aggre2_ufs_axi", @@ -1516,8 +1512,8 @@ static const struct qcom_icc_desc sdm660_a2noc = { .type = QCOM_ICC_NOC, .nodes = sdm660_a2noc_nodes, .num_nodes = ARRAY_SIZE(sdm660_a2noc_nodes), - .bus_clocks = bus_a2noc_clocks, - .num_bus_clocks = ARRAY_SIZE(bus_a2noc_clocks), + .intf_clocks = a2noc_intf_clocks, + .num_intf_clocks = ARRAY_SIZE(a2noc_intf_clocks), .regmap_cfg = &sdm660_a2noc_regmap_config, }; @@ -1659,8 +1655,8 @@ static const struct qcom_icc_desc sdm660_mnoc = { .type = QCOM_ICC_NOC, .nodes = sdm660_mnoc_nodes, .num_nodes = ARRAY_SIZE(sdm660_mnoc_nodes), - .bus_clocks = bus_mm_clocks, - .num_bus_clocks = ARRAY_SIZE(bus_mm_clocks), + .intf_clocks = mm_intf_clocks, + .num_intf_clocks = ARRAY_SIZE(mm_intf_clocks), .regmap_cfg = &sdm660_mnoc_regmap_config, };
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. 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@linaro.org> --- 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(-)