Re: [PATCH v2 2/3] interconnect: qcom: sdm660: Add missing a2noc qos clocks

[Bjorn Andersson <bjorn.andersson@linaro.org>]

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
> > > 
>