| Message ID | 20220415005828.1980055-1-swboyd@chromium.org |
|---|---|
| State | Accepted |
| Commit | 067bc653b85e466048914c48e46659a50a907fa6 |
| Headers | show |
| Series | interconnect: qcom: Remove IP0 resource | expand |
On 4/14/22 7:58 PM, 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. Look good. Reviewed-by: Alex Elder <elder@linaro.org> > Cc: Alex Elder <elder@linaro.org> > Cc: Taniya Das <quic_tdas@quicinc.com> > Cc: Mike Tipton <quic_mdtipton@quicinc.com> > Cc: Georgi Djakov <djakov@kernel.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > arch/arm/boot/dts/qcom-sdx55.dtsi | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom-sdx55.dtsi > index d455795da44c..f1e12a112cd0 100644 > --- a/arch/arm/boot/dts/qcom-sdx55.dtsi > +++ b/arch/arm/boot/dts/qcom-sdx55.dtsi > @@ -275,13 +275,6 @@ system_noc: interconnect@162c000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > - ipa_virt: interconnect@1e00000 { > - compatible = "qcom,sdx55-ipa-virt"; > - reg = <0x01e00000 0x100000>; > - #interconnect-cells = <1>; > - qcom,bcm-voters = <&apps_bcm_voter>; > - }; > - > qpic_bam: dma-controller@1b04000 { > compatible = "qcom,bam-v1.7.0"; > reg = <0x01b04000 0x1c000>;
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... Regards, Bjorn > Cc: Alex Elder <elder@linaro.org> > Cc: Taniya Das <quic_tdas@quicinc.com> > Cc: Mike Tipton <quic_mdtipton@quicinc.com> > Cc: Georgi Djakov <djakov@kernel.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index e1c46b80f14a..1ff96ef30e3f 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -1421,13 +1421,6 @@ mmss_noc: interconnect@1740000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > - ipa_virt: interconnect@1e00000 { > - compatible = "qcom,sc7180-ipa-virt"; > - reg = <0 0x01e00000 0 0x1000>; > - #interconnect-cells = <2>; > - qcom,bcm-voters = <&apps_bcm_voter>; > - }; > - > ipa: ipa@1e40000 { > compatible = "qcom,sc7180-ipa"; > > -- > https://chromeos.dev >
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... 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. >> Cc: Alex Elder <elder@linaro.org> >> Cc: Taniya Das <quic_tdas@quicinc.com> >> Cc: Mike Tipton <quic_mdtipton@quicinc.com> >> Cc: Georgi Djakov <djakov@kernel.org> >> Signed-off-by: Stephen Boyd <swboyd@chromium.org> Acked-by: Georgi Djakov <djakov@kernel.org> Thanks, Georgi >> --- >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> index e1c46b80f14a..1ff96ef30e3f 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> @@ -1421,13 +1421,6 @@ mmss_noc: interconnect@1740000 { >> qcom,bcm-voters = <&apps_bcm_voter>; >> }; >> >> - ipa_virt: interconnect@1e00000 { >> - compatible = "qcom,sc7180-ipa-virt"; >> - reg = <0 0x01e00000 0 0x1000>; >> - #interconnect-cells = <2>; >> - qcom,bcm-voters = <&apps_bcm_voter>; >> - }; >> - >> ipa: ipa@1e40000 { >> compatible = "qcom,sc7180-ipa"; >> >> -- >> https://chromeos.dev >>
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); }
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index e1c46b80f14a..1ff96ef30e3f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1421,13 +1421,6 @@ mmss_noc: interconnect@1740000 { qcom,bcm-voters = <&apps_bcm_voter>; }; - ipa_virt: interconnect@1e00000 { - compatible = "qcom,sc7180-ipa-virt"; - reg = <0 0x01e00000 0 0x1000>; - #interconnect-cells = <2>; - qcom,bcm-voters = <&apps_bcm_voter>; - }; - ipa: ipa@1e40000 { compatible = "qcom,sc7180-ipa";
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. Cc: Alex Elder <elder@linaro.org> Cc: Taniya Das <quic_tdas@quicinc.com> Cc: Mike Tipton <quic_mdtipton@quicinc.com> Cc: Georgi Djakov <djakov@kernel.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 ------- 1 file changed, 7 deletions(-)