| Message ID | 20210824043435.23190-3-shawn.guo@linaro.org |
|---|---|
| State | Accepted |
| Commit | 13404ac8882f5225af07545215f4975a564c3740 |
| Headers | show |
| Series | Add missing A2NoC QoS clocks for SDM660 interconnect driver | expand |
Il 24/08/21 06:34, Shawn Guo ha scritto: > It adds the missing a2noc clocks required for QoS registers programming > per downstream kernel[1]. Otherwise, qcom_icc_noc_set_qos_priority() > call on mas_ufs or mas_usb_hs node will simply result in a hardware hang > on SDM660 SoC. > > [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43 > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org> > My Xperia devices are not locking up on any call to qos priority setting, but that's surely because such clocks are left on by the bootloader before booting Linux... and then after booting my USB is up, which is probably why I still don't experience any hang, I guess. In any case, I do recognize that the a2noc does effectively need these clocks to be on in order to be reachable, as also reported in the downstream dtsi. Thank you! Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
On Mon 23 Aug 23:34 CDT 2021, Shawn Guo wrote: > It adds the missing a2noc clocks required for QoS registers programming > per downstream kernel[1]. Otherwise, qcom_icc_noc_set_qos_priority() > call on mas_ufs or mas_usb_hs node will simply result in a hardware hang > on SDM660 SoC. > > [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43 > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org> Georgi, do you intend to pull this patch in for v5.15-rc? I.e. should I pick up the dts change for v5.15 as well. Regards, Bjorn > --- > drivers/interconnect/qcom/sdm660.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c > index c89c991a80a0..661eb3635d21 100644 > --- a/drivers/interconnect/qcom/sdm660.c > +++ b/drivers/interconnect/qcom/sdm660.c > @@ -174,6 +174,16 @@ static const struct clk_bulk_data bus_mm_clocks[] = { > { .id = "iface" }, > }; > > +static const struct clk_bulk_data bus_a2noc_clocks[] = { > + { .id = "bus" }, > + { .id = "bus_a" }, > + { .id = "ipa" }, > + { .id = "ufs_axi" }, > + { .id = "aggre2_ufs_axi" }, > + { .id = "aggre2_usb3_axi" }, > + { .id = "cfg_noc_usb2_axi" }, > +}; > + > /** > * struct qcom_icc_provider - Qualcomm specific interconnect provider > * @provider: generic interconnect provider > @@ -811,6 +821,10 @@ static int qnoc_probe(struct platform_device *pdev) > qp->bus_clks = devm_kmemdup(dev, bus_mm_clocks, > sizeof(bus_mm_clocks), GFP_KERNEL); > qp->num_clks = ARRAY_SIZE(bus_mm_clocks); > + } else if (of_device_is_compatible(dev->of_node, "qcom,sdm660-a2noc")) { > + qp->bus_clks = devm_kmemdup(dev, bus_a2noc_clocks, > + sizeof(bus_a2noc_clocks), GFP_KERNEL); > + qp->num_clks = ARRAY_SIZE(bus_a2noc_clocks); > } else { > if (of_device_is_compatible(dev->of_node, "qcom,sdm660-bimc")) > qp->is_bimc_node = true; > -- > 2.17.1 >
On Fri 24 Sep 07:58 PDT 2021, Georgi Djakov wrote: > On 24.09.21 17:33, Bjorn Andersson wrote: > > On Mon 23 Aug 23:34 CDT 2021, Shawn Guo wrote: > > > > > It adds the missing a2noc clocks required for QoS registers programming > > > per downstream kernel[1]. Otherwise, qcom_icc_noc_set_qos_priority() > > > call on mas_ufs or mas_usb_hs node will simply result in a hardware hang > > > on SDM660 SoC. > > > > > > [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/sdm660-bus.dtsi?h=LA.UM.8.2.r1-04800-sdm660.0#n43 > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > Georgi, do you intend to pull this patch in for v5.15-rc? > > > > I.e. should I pick up the dts change for v5.15 as well. > > Yes, i have just sent a pull request with this included, so please > pull the dts change as a fix for v5.15-rc. Sorry for not mentioning > it explicitly. > I assume that not all bootloaders are leaving the qos clocks enabled, > so if this is fixing a hardware hang, should we also backport it into > stable? This is probably more of a question to the people actively > using this board? > Devices that needed this change wouldn't boot on that older kernel anyways, so I think it's safe to assume that we don't have any users on an old kernel that would benefit from this change. So v5.15 is probably sufficient. Regards, Bjorn > Thanks, > Georgi > > > Regards, > > Bjorn > > > > > --- > > > drivers/interconnect/qcom/sdm660.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c > > > index c89c991a80a0..661eb3635d21 100644 > > > --- a/drivers/interconnect/qcom/sdm660.c > > > +++ b/drivers/interconnect/qcom/sdm660.c > > > @@ -174,6 +174,16 @@ static const struct clk_bulk_data bus_mm_clocks[] = { > > > { .id = "iface" }, > > > }; > > > +static const struct clk_bulk_data bus_a2noc_clocks[] = { > > > + { .id = "bus" }, > > > + { .id = "bus_a" }, > > > + { .id = "ipa" }, > > > + { .id = "ufs_axi" }, > > > + { .id = "aggre2_ufs_axi" }, > > > + { .id = "aggre2_usb3_axi" }, > > > + { .id = "cfg_noc_usb2_axi" }, > > > +}; > > > + > > > /** > > > * struct qcom_icc_provider - Qualcomm specific interconnect provider > > > * @provider: generic interconnect provider > > > @@ -811,6 +821,10 @@ static int qnoc_probe(struct platform_device *pdev) > > > qp->bus_clks = devm_kmemdup(dev, bus_mm_clocks, > > > sizeof(bus_mm_clocks), GFP_KERNEL); > > > qp->num_clks = ARRAY_SIZE(bus_mm_clocks); > > > + } else if (of_device_is_compatible(dev->of_node, "qcom,sdm660-a2noc")) { > > > + qp->bus_clks = devm_kmemdup(dev, bus_a2noc_clocks, > > > + sizeof(bus_a2noc_clocks), GFP_KERNEL); > > > + qp->num_clks = ARRAY_SIZE(bus_a2noc_clocks); > > > } else { > > > if (of_device_is_compatible(dev->of_node, "qcom,sdm660-bimc")) > > > qp->is_bimc_node = true; > > > -- > > > 2.17.1 > > > >
diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c index c89c991a80a0..661eb3635d21 100644 --- a/drivers/interconnect/qcom/sdm660.c +++ b/drivers/interconnect/qcom/sdm660.c @@ -174,6 +174,16 @@ static const struct clk_bulk_data bus_mm_clocks[] = { { .id = "iface" }, }; +static const struct clk_bulk_data bus_a2noc_clocks[] = { + { .id = "bus" }, + { .id = "bus_a" }, + { .id = "ipa" }, + { .id = "ufs_axi" }, + { .id = "aggre2_ufs_axi" }, + { .id = "aggre2_usb3_axi" }, + { .id = "cfg_noc_usb2_axi" }, +}; + /** * struct qcom_icc_provider - Qualcomm specific interconnect provider * @provider: generic interconnect provider @@ -811,6 +821,10 @@ static int qnoc_probe(struct platform_device *pdev) qp->bus_clks = devm_kmemdup(dev, bus_mm_clocks, sizeof(bus_mm_clocks), GFP_KERNEL); qp->num_clks = ARRAY_SIZE(bus_mm_clocks); + } else if (of_device_is_compatible(dev->of_node, "qcom,sdm660-a2noc")) { + qp->bus_clks = devm_kmemdup(dev, bus_a2noc_clocks, + sizeof(bus_a2noc_clocks), GFP_KERNEL); + qp->num_clks = ARRAY_SIZE(bus_a2noc_clocks); } else { if (of_device_is_compatible(dev->of_node, "qcom,sdm660-bimc")) qp->is_bimc_node = true;