2022-04-13 06:54:40

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 0/2] interconnect: qcom: Remove IP0 resource

These two patches remove the IP0 interconnect used for IPA because
they're also present in the clk-rpmh driver. I see there are some more
IP0 usages in the interconnect drivers, but I don't see a corresponding
IPA clk in clk-rpmh, so I left these out. We can remove all of them if
desired, but the sc7180 patch is most important to me as it fixes
boot on my trogdor lazor device.

Stephen Boyd (2):
interconnect: qcom: sc7180: Drop IP0 interconnects
interconnect: qcom: sdx55: Drop IP0 interconnects

drivers/interconnect/qcom/sc7180.c | 21 ---------------------
drivers/interconnect/qcom/sdx55.c | 21 ---------------------
2 files changed, 42 deletions(-)

Cc: Alex Elder <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Taniya Das <[email protected]>
Cc: Mike Tipton <[email protected]>
Cc: Manivannan Sadhasivam <[email protected]>

base-commit: 3123109284176b1532874591f7c81f3837bbdc17
--
https://chromeos.dev


2022-04-14 07:54:21

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 0/2] interconnect: qcom: Remove IP0 resource

On Tue 12 Apr 15:00 PDT 2022, Stephen Boyd wrote:

> These two patches remove the IP0 interconnect used for IPA because
> they're also present in the clk-rpmh driver. I see there are some more
> IP0 usages in the interconnect drivers, but I don't see a corresponding
> IPA clk in clk-rpmh, so I left these out. We can remove all of them if
> desired, but the sc7180 patch is most important to me as it fixes
> boot on my trogdor lazor device.
>
> Stephen Boyd (2):
> interconnect: qcom: sc7180: Drop IP0 interconnects
> interconnect: qcom: sdx55: Drop IP0 interconnects
>
> drivers/interconnect/qcom/sc7180.c | 21 ---------------------
> drivers/interconnect/qcom/sdx55.c | 21 ---------------------
> 2 files changed, 42 deletions(-)
>
> Cc: Alex Elder <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Taniya Das <[email protected]>
> Cc: Mike Tipton <[email protected]>
> Cc: Manivannan Sadhasivam <[email protected]>
>

Reviewed-by: Bjorn Andersson <[email protected]>

> base-commit: 3123109284176b1532874591f7c81f3837bbdc17
> --
> https://chromeos.dev
>

2022-04-16 02:29:18

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node

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 <[email protected]>
Cc: Taniya Das <[email protected]>
Cc: Mike Tipton <[email protected]>
Cc: Georgi Djakov <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
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

2022-04-16 02:44:12

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 5/2] dt-bindings: interconnect: Remove sc7180/sdx55 ipa compatibles

These interconnects are modeled as clks, not interconnects, therefore
remove the compatibles from the binding as they're unused.

Cc: Alex Elder <[email protected]>
Cc: Taniya Das <[email protected]>
Cc: Mike Tipton <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---

I don't know who should apply this. Probably whoever takes the dtsi
patches, Bjorn?, because otherwise dt_bindings_check will fail.

Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml | 2 --
1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
index 5a911be0c2ea..ab859150c7f7 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
@@ -31,7 +31,6 @@ properties:
- qcom,sc7180-config-noc
- qcom,sc7180-dc-noc
- qcom,sc7180-gem-noc
- - qcom,sc7180-ipa-virt
- qcom,sc7180-mc-virt
- qcom,sc7180-mmss-noc
- qcom,sc7180-npu-noc
@@ -68,7 +67,6 @@ properties:
- qcom,sdm845-mem-noc
- qcom,sdm845-mmss-noc
- qcom,sdm845-system-noc
- - qcom,sdx55-ipa-virt
- qcom,sdx55-mc-virt
- qcom,sdx55-mem-noc
- qcom,sdx55-system-noc
--
https://chromeos.dev

2022-04-21 00:24:46

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node

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 <[email protected]>
> Cc: Taniya Das <[email protected]>
> Cc: Mike Tipton <[email protected]>
> Cc: Georgi Djakov <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> 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
>

2022-04-22 21:56:35

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node

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 <[email protected]>
>> Cc: Taniya Das <[email protected]>
>> Cc: Mike Tipton <[email protected]>
>> Cc: Georgi Djakov <[email protected]>
>> Signed-off-by: Stephen Boyd <[email protected]>

Acked-by: Georgi Djakov <[email protected]>

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

2022-04-22 23:04:20

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node

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);
}

2022-05-05 08:56:56

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH 3/2] arm64: dts: qcom: sc7180: Remove ipa interconnect node

On Thu, 14 Apr 2022 17:58:26 -0700, 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.
>
>

Applied, thanks!

[3/3] arm64: dts: qcom: sc7180: Remove ipa interconnect node
commit: 067bc653b85e466048914c48e46659a50a907fa6

Best regards,
--
Bjorn Andersson <[email protected]>

2022-05-16 23:40:35

by Alex Elder

[permalink] [raw]
Subject: Re: [PATCH 5/2] dt-bindings: interconnect: Remove sc7180/sdx55 ipa compatibles

On 4/14/22 7:58 PM, Stephen Boyd wrote:
> These interconnects are modeled as clks, not interconnects, therefore
> remove the compatibles from the binding as they're unused.
>
> Cc: Alex Elder <[email protected]>
> Cc: Taniya Das <[email protected]>
> Cc: Mike Tipton <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
>
> I don't know who should apply this. Probably whoever takes the dtsi
> patches, Bjorn?, because otherwise dt_bindings_check will fail.

I don't see this commit applied anywhere, though I
might have missed it. Is this for Bjorn, or Georgi,
or someone else?

-Alex

> Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
> index 5a911be0c2ea..ab859150c7f7 100644
> --- a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
> @@ -31,7 +31,6 @@ properties:
> - qcom,sc7180-config-noc
> - qcom,sc7180-dc-noc
> - qcom,sc7180-gem-noc
> - - qcom,sc7180-ipa-virt
> - qcom,sc7180-mc-virt
> - qcom,sc7180-mmss-noc
> - qcom,sc7180-npu-noc
> @@ -68,7 +67,6 @@ properties:
> - qcom,sdm845-mem-noc
> - qcom,sdm845-mmss-noc
> - qcom,sdm845-system-noc
> - - qcom,sdx55-ipa-virt
> - qcom,sdx55-mc-virt
> - qcom,sdx55-mem-noc
> - qcom,sdx55-system-noc


2022-05-20 08:42:50

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH 5/2] dt-bindings: interconnect: Remove sc7180/sdx55 ipa compatibles

On 17.05.22 1:16, Alex Elder wrote:
> On 4/14/22 7:58 PM, Stephen Boyd wrote:
>> These interconnects are modeled as clks, not interconnects, therefore
>> remove the compatibles from the binding as they're unused.
>>
>> Cc: Alex Elder <[email protected]>
>> Cc: Taniya Das <[email protected]>
>> Cc: Mike Tipton <[email protected]>
>> Signed-off-by: Stephen Boyd <[email protected]>
>> ---
>>
>> I don't know who should apply this. Probably whoever takes the dtsi
>> patches, Bjorn?, because otherwise dt_bindings_check will fail.
>
> I don't see this commit applied anywhere, though I
> might have missed it.  Is this for Bjorn, or Georgi,
> or someone else?

I merged it as Bjorn has already sent his pull request.

Thanks,
Georgi

>
>                     -Alex
>
>>   Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
>> b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
>> index 5a911be0c2ea..ab859150c7f7 100644
>> --- a/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml
>> @@ -31,7 +31,6 @@ properties:
>>         - qcom,sc7180-config-noc
>>         - qcom,sc7180-dc-noc
>>         - qcom,sc7180-gem-noc
>> -      - qcom,sc7180-ipa-virt
>>         - qcom,sc7180-mc-virt
>>         - qcom,sc7180-mmss-noc
>>         - qcom,sc7180-npu-noc
>> @@ -68,7 +67,6 @@ properties:
>>         - qcom,sdm845-mem-noc
>>         - qcom,sdm845-mmss-noc
>>         - qcom,sdm845-system-noc
>> -      - qcom,sdx55-ipa-virt
>>         - qcom,sdx55-mc-virt
>>         - qcom,sdx55-mem-noc
>>         - qcom,sdx55-system-noc
>