Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
In-Reply-To: <e54804ee-15f8-36a1-bbc5-9a218a4aec56@kernel.org>
References: <20220412220033.1273607-1-swboyd@chromium.org> <20220415005828.1980055-1-swboyd@chromium.org>
 <Yl92UkRwlQsd71mr@ripper> <e54804ee-15f8-36a1-bbc5-9a218a4aec56@kernel.org>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date:   Fri, 22 Apr 2022 13:34:36 -0700
Message-ID: <CAE-0n50OvNmcX9fir8+zgaz9sE+sksNzTjN3vWgzT3A7ay6s+A@mail.gmail.com>
Subject: Re: [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node
To:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Georgi Djakov <djakov@kernel.org>
Cc:     linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
        Doug Anderson <dianders@chromium.org>,
        Alex Elder <elder@linaro.org>,
        Taniya Das <quic_tdas@quicinc.com>,
        Mike Tipton <quic_mdtipton@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Georgi Djakov (2022-04-22 00:01:24)
> On 20.04.22 5:56, Bjorn Andersson wrote:
> > On Thu 14 Apr 17:58 PDT 2022, Stephen Boyd wrote:
> >
> >> This device node is unused now that we've removed the driver that
> >> consumed it in the kernel. Drop the unused node to save some space.
> >>
> >
> > I'm expecting that merging patch 3 and 4 will work, but cause sync_state
> > to not happen until the driver changes are merged.
> >
> > Can you confirm my expectation? And perhaps confirm that it's fine for
> > Georgi to pick the driver changes independently of the dts changes...

It should be OK to pick the driver changes independently of the dts.
They fix a boot up issue.

>
> I have picked the driver changes, as the boot failure definitely needs to
> be addressed. The sync-state might not happen until we have the DT changes
> merged, as the framework is matching the count of probed drivers with the
> count of providers in DT.

Indeed. The DT change is required to actually have sync-state happen
when the driver changes are merged. Without the DT change I'm not able
to enter suspend. I didn't notice this earlier, ugh.

This means that the DTS change needs to be backported to fix suspend,
because otherwise the _other_ interconnects that aren't IPA keep the
initial sync-state request forever, waiting for the IPA provider in DT
to be probed by a driver that doesn't exist. One solution is to have the
DT change, which makes the probed driver and provider counts match.

Maybe a better solution is to ignore these compatibles in the provider
count? That way we aren't required to backport a DTS change and
everything is still contained to driver code.

----8<----
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 80ed03f4dfd0..bfa6788afca1 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -1087,9 +1087,15 @@ static int of_count_icc_providers(struct device_node *np)
 {
 	struct device_node *child;
 	int count = 0;
+	const struct of_device_id ignore_list[] = {
+		{ .compatible = "qcom,sc7180-ipa-virt", },
+		{ .compatible = "qcom,sdx55-ipa-virt", },
+		{}
+	};

 	for_each_available_child_of_node(np, child) {
-		if (of_property_read_bool(child, "#interconnect-cells"))
+		if (of_property_read_bool(child, "#interconnect-cells") &&
+		    likely(!of_match_node(ignore_list, child)))
 			count++;
 		count += of_count_icc_providers(child);
 	}