After the recent cleanups ([1], [2]) some in-tree abusers that directly
accessed the RPM bus clocks, effectively circumventing and working
against the efforts of the interconnect framework, were found.
Patches 1-5 drop deprecated references and the rest attempt to stop
direct bus clock abuses.
Depends on [2].
8996 and 8998 remoteproc changes were not tested, they never worked on
my Sony phones.
[1] https://lore.kernel.org/linux-arm-msm/[email protected]/
[2] https://lore.kernel.org/linux-arm-msm/[email protected]/
Signed-off-by: Konrad Dybcio <[email protected]>
---
Changes in v2:
- Incorporate [3] into the sdm630 patch, add required bindings fixes
- dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock: Merge entries (krzk)
- Pick up a-b (krzk)
- Add "sdm630: Fix USB2 clock-names order"
- Link to v1: https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/linux-arm-msm/[email protected]/#t
---
Konrad Dybcio (14):
arm64: dts: qcom: msm8916: Drop RPM bus clocks
arm64: dts: qcom: msm8996: Drop RPM bus clocks
arm64: dts: qcom: qcs404: Drop RPM bus clocks
dt-bindings: arm-smmu: Fix SDM630 clocks description
dt-bindings: usb: qcom,dwc3: Fix SDM660 clock description
arm64: dts: qcom: sdm630: Drop RPM bus clocks
arm64: dts: qcom: msm8939: Drop RPM bus clocks
dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock
dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock
remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS
arm64: dts: qcom: msm8998: Remove AGGRE2 clock from SLPI
arm64: dts: qcom: msm8996: Remove AGGRE2 clock from SLPI
arm64: dts: qcom: msm8996: Remove PNoC clock from MSS
arm64: dts: qcom: sdm630: Fix USB2 clock-names order
.../devicetree/bindings/iommu/arm,smmu.yaml | 2 +-
.../devicetree/bindings/remoteproc/qcom,adsp.yaml | 20 +-------
.../bindings/remoteproc/qcom,msm8996-mss-pil.yaml | 2 -
.../devicetree/bindings/usb/qcom,dwc3.yaml | 6 +--
arch/arm64/boot/dts/qcom/msm8916.dtsi | 9 ----
arch/arm64/boot/dts/qcom/msm8939.dtsi | 12 -----
arch/arm64/boot/dts/qcom/msm8996.dtsi | 43 ++++++-----------
arch/arm64/boot/dts/qcom/msm8998.dtsi | 5 +-
arch/arm64/boot/dts/qcom/qcs404.dtsi | 9 ----
arch/arm64/boot/dts/qcom/sdm630.dtsi | 55 +++++-----------------
drivers/remoteproc/qcom_q6v5_mss.c | 1 -
11 files changed, 34 insertions(+), 130 deletions(-)
---
base-commit: 66d9573193967138cd12e232d4b5bc2b57e0d1ac
change-id: 20230721-topic-rpm_clk_cleanup-1b2f4a1acd01
Best regards,
--
Konrad Dybcio <[email protected]>
The last 2 clock-names entries for the USB2 controller were swapped,
resulting in schema warnings:
['cfg_noc', 'core', 'mock_utmi', 'sleep'] is too short
'iface' was expected
'sleep' was expected
'mock_utmi' was expected
Fix it and take the liberty to make the clock-names entries more
readable.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index f11d2a07508c..316c8fd224e0 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1394,8 +1394,10 @@ usb2: usb@c2f8800 {
<&gcc GCC_USB20_MASTER_CLK>,
<&gcc GCC_USB20_MOCK_UTMI_CLK>,
<&gcc GCC_USB20_SLEEP_CLK>;
- clock-names = "cfg_noc", "core",
- "mock_utmi", "sleep";
+ clock-names = "cfg_noc",
+ "core",
+ "sleep",
+ "mock_utmi";
assigned-clocks = <&gcc GCC_USB20_MOCK_UTMI_CLK>,
<&gcc GCC_USB20_MASTER_CLK>;
--
2.42.0
These clocks are now handled from within the icc framework and are
no longer registered from within the CCF. Remove them.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 2721f32dfb71..317a0df30b83 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -558,9 +558,6 @@ bimc: interconnect@400000 {
reg = <0x00400000 0x80000>;
compatible = "qcom,qcs404-bimc";
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_BIMC_CLK>,
- <&rpmcc RPM_SMD_BIMC_A_CLK>;
};
tsens: thermal-sensor@4a9000 {
@@ -601,18 +598,12 @@ pcnoc: interconnect@500000 {
reg = <0x00500000 0x15080>;
compatible = "qcom,qcs404-pcnoc";
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_PNOC_CLK>,
- <&rpmcc RPM_SMD_PNOC_A_CLK>;
};
snoc: interconnect@580000 {
reg = <0x00580000 0x23080>;
compatible = "qcom,qcs404-snoc";
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
- <&rpmcc RPM_SMD_SNOC_A_CLK>;
};
remoteproc_cdsp: remoteproc@b00000 {
--
2.42.0
These clocks are now handled from within the icc framework and are
no longer registered from within the CCF. Remove them.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 33fb65d73104..8c2ec09f57c4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -519,9 +519,6 @@ bimc: interconnect@400000 {
compatible = "qcom,msm8916-bimc";
reg = <0x00400000 0x62000>;
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_BIMC_CLK>,
- <&rpmcc RPM_SMD_BIMC_A_CLK>;
};
tsens: thermal-sensor@4a9000 {
@@ -554,18 +551,12 @@ pcnoc: interconnect@500000 {
compatible = "qcom,msm8916-pcnoc";
reg = <0x00500000 0x11000>;
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_PCNOC_CLK>,
- <&rpmcc RPM_SMD_PCNOC_A_CLK>;
};
snoc: interconnect@580000 {
compatible = "qcom,msm8916-snoc";
reg = <0x00580000 0x14000>;
#interconnect-cells = <1>;
- clock-names = "bus", "bus_a";
- clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
- <&rpmcc RPM_SMD_SNOC_A_CLK>;
};
stm: stm@802000 {
--
2.42.0
SDM660 was abusingly referencing one of the internal bus clocks, that
were recently dropped from Linux (because the original implementation
did not make much sense), circumventing the interconnect framework.
Drop it.
Signed-off-by: Konrad Dybcio <[email protected]>
---
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 67591057f234..820878031d47 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -283,8 +283,8 @@ allOf:
then:
properties:
clocks:
- minItems: 5
- maxItems: 6
+ minItems: 4
+ maxItems: 5
clock-names:
oneOf:
- items:
@@ -293,13 +293,11 @@ allOf:
- const: iface
- const: sleep
- const: mock_utmi
- - const: bus
- items:
- const: cfg_noc
- const: core
- const: sleep
- const: mock_utmi
- - const: bus
- if:
properties:
--
2.42.0
On 12/09/2023 15:31, Konrad Dybcio wrote:
> These clocks are now handled from within the icc framework and are
That's a driver behavior, not hardware.
> no longer registered from within the CCF. Remove them.
>
Changes in Linux clock drivers should not cause some clocks to disappear
from DTS...
Best regards,
Krzysztof
On 13.09.2023 10:53, Krzysztof Kozlowski wrote:
> On 13/09/2023 10:47, Konrad Dybcio wrote:
>> On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
>>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>>> These clocks are now handled from within the icc framework and are
>>>
>>> That's a driver behavior, not hardware.
>> I believe we've been over this already..
>>
>> The rationale behind this change is: that hardware, which falls
>> under the "interconnect" class, was previously misrepresented as
>> a bunch of clocks. There are clocks underneath, but accessing them
>> directly would be equivalent to e.g. circumventing the PHY subsystem
>> and initializing your UFS PHY from within the UFS device.
>
> And every time one write such commit msg, how should we remember there
> is some exception and actually it is about clock representation not CCF
> or ICC framework.
So is your reply essentially "fine, but please make it clear in
each commit message"?
Konrad
On 12/09/2023 15:31, Konrad Dybcio wrote:
> The last 2 clock-names entries for the USB2 controller were swapped,
> resulting in schema warnings:
>
> ['cfg_noc', 'core', 'mock_utmi', 'sleep'] is too short
> 'iface' was expected
> 'sleep' was expected
> 'mock_utmi' was expected
>
> Fix it and take the liberty to make the clock-names entries more
> readable.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sdm630.dtsi | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index f11d2a07508c..316c8fd224e0 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -1394,8 +1394,10 @@ usb2: usb@c2f8800 {
> <&gcc GCC_USB20_MASTER_CLK>,
> <&gcc GCC_USB20_MOCK_UTMI_CLK>,
> <&gcc GCC_USB20_SLEEP_CLK>;
> - clock-names = "cfg_noc", "core",
> - "mock_utmi", "sleep";
> + clock-names = "cfg_noc",
> + "core",
> + "sleep",
> + "mock_utmi";
Plus this is just incorrect... :(
Best regards,
Krzysztof
SDM630 was abusingly referencing one of the internal bus clocks, that
were recently dropped from Linux (because the original implementation
did not make much sense), circumventing the interconnect framework.
Fix it by dropping the bus-mm clock (which requires separating 630 from
similar entries) and keeping the rest as-is.
Signed-off-by: Konrad Dybcio <[email protected]>
---
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index cf29ab10501c..b1b2cf81b42f 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -270,6 +270,7 @@ allOf:
contains:
enum:
- qcom,msm8998-smmu-v2
+ - qcom,sdm630-smmu-v2
then:
anyOf:
- properties:
@@ -311,7 +312,6 @@ allOf:
compatible:
contains:
enum:
- - qcom,sdm630-smmu-v2
- qcom,sm6375-smmu-v2
then:
anyOf:
--
2.42.0
On 13.09.2023 13:14, Krzysztof Kozlowski wrote:
> On 13/09/2023 12:48, Konrad Dybcio wrote:
>> On 13.09.2023 10:53, Krzysztof Kozlowski wrote:
>>> On 13/09/2023 10:47, Konrad Dybcio wrote:
>>>> On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
>>>>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>>>>> These clocks are now handled from within the icc framework and are
>>>>>
>>>>> That's a driver behavior, not hardware.
>>>> I believe we've been over this already..
>>>>
>>>> The rationale behind this change is: that hardware, which falls
>>>> under the "interconnect" class, was previously misrepresented as
>>>> a bunch of clocks. There are clocks underneath, but accessing them
>>>> directly would be equivalent to e.g. circumventing the PHY subsystem
>>>> and initializing your UFS PHY from within the UFS device.
>>>
>>> And every time one write such commit msg, how should we remember there
>>> is some exception and actually it is about clock representation not CCF
>>> or ICC framework.
>> So is your reply essentially "fine, but please make it clear in
>> each commit message"?
>
> I am fine with this change. If commit msg had such statement, I would
> not have doubts :/
Ok, I'll resend, thanks for confirming!
Konrad
On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
> On 12/09/2023 15:31, Konrad Dybcio wrote:
>> These clocks are now handled from within the icc framework and are
>
> That's a driver behavior, not hardware.
I believe we've been over this already..
The rationale behind this change is: that hardware, which falls
under the "interconnect" class, was previously misrepresented as
a bunch of clocks. There are clocks underneath, but accessing them
directly would be equivalent to e.g. circumventing the PHY subsystem
and initializing your UFS PHY from within the UFS device.
Konrad
>
>> no longer registered from within the CCF. Remove them.
>>
>
> Changes in Linux clock drivers should not cause some clocks to disappear
> from DTS...
>
>
> Best regards,
> Krzysztof
>
On 13/09/2023 10:47, Konrad Dybcio wrote:
> On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>> These clocks are now handled from within the icc framework and are
>>
>> That's a driver behavior, not hardware.
> I believe we've been over this already..
>
> The rationale behind this change is: that hardware, which falls
> under the "interconnect" class, was previously misrepresented as
> a bunch of clocks. There are clocks underneath, but accessing them
> directly would be equivalent to e.g. circumventing the PHY subsystem
> and initializing your UFS PHY from within the UFS device.
And every time one write such commit msg, how should we remember there
is some exception and actually it is about clock representation not CCF
or ICC framework.
Best regards,
Krzysztof
On 12/09/2023 15:31, Konrad Dybcio wrote:
> The last 2 clock-names entries for the USB2 controller were swapped,
> resulting in schema warnings:
>
> ['cfg_noc', 'core', 'mock_utmi', 'sleep'] is too short
> 'iface' was expected
> 'sleep' was expected
> 'mock_utmi' was expected
>
> Fix it and take the liberty to make the clock-names entries more
> readable.
This was already fixed:
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
On 13/09/2023 12:48, Konrad Dybcio wrote:
> On 13.09.2023 10:53, Krzysztof Kozlowski wrote:
>> On 13/09/2023 10:47, Konrad Dybcio wrote:
>>> On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
>>>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>>>> These clocks are now handled from within the icc framework and are
>>>>
>>>> That's a driver behavior, not hardware.
>>> I believe we've been over this already..
>>>
>>> The rationale behind this change is: that hardware, which falls
>>> under the "interconnect" class, was previously misrepresented as
>>> a bunch of clocks. There are clocks underneath, but accessing them
>>> directly would be equivalent to e.g. circumventing the PHY subsystem
>>> and initializing your UFS PHY from within the UFS device.
>>
>> And every time one write such commit msg, how should we remember there
>> is some exception and actually it is about clock representation not CCF
>> or ICC framework.
> So is your reply essentially "fine, but please make it clear in
> each commit message"?
I am fine with this change. If commit msg had such statement, I would
not have doubts :/
Best regards,
Krzysztof
The AGGRE2 clock is a clock for the entire AGGRE2 bus, managed from
within the interconnect driver. Attaching it to SLPI was a total hack.
Get rid of it.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index acef67ab0581..7061a8e12c81 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -2433,9 +2433,8 @@ slpi_pil: remoteproc@1c00000 {
"handover",
"stop-ack";
- clocks = <&xo_board>,
- <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
- clock-names = "xo", "aggre2";
+ clocks = <&xo_board>;
+ clock-names = "xo";
memory-region = <&slpi_mem>;
--
2.42.0
On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote:
> After the recent cleanups ([1], [2]) some in-tree abusers that directly
> accessed the RPM bus clocks, effectively circumventing and working
> against the efforts of the interconnect framework, were found.
>
> Patches 1-5 drop deprecated references and the rest attempt to stop
> direct bus clock abuses.
>
> [...]
Applied SMMU bindings fix to will (for-joerg/arm-smmu/fixes), thanks!
[04/14] dt-bindings: arm-smmu: Fix SDM630 clocks description
https://git.kernel.org/will/c/938ba2f252a5
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote:
> After the recent cleanups ([1], [2]) some in-tree abusers that directly
> accessed the RPM bus clocks, effectively circumventing and working
> against the efforts of the interconnect framework, were found.
>
> Patches 1-5 drop deprecated references and the rest attempt to stop
> direct bus clock abuses.
>
> [...]
Applied, thanks!
[08/14] dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock
commit: c4c5b47958529bc1de10260df0c583710853b516
[09/14] dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock
commit: e7781901449cbcff129d80a5d9021e9e96084ec4
[10/14] remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS
commit: e1592981c51bac38ea2041b642777b3ba30606a8
Best regards,
--
Bjorn Andersson <[email protected]>
On 9/13/2023 7:14 PM, Konrad Dybcio wrote:
> On 13.09.2023 13:14, Krzysztof Kozlowski wrote:
>> On 13/09/2023 12:48, Konrad Dybcio wrote:
>>> On 13.09.2023 10:53, Krzysztof Kozlowski wrote:
>>>> On 13/09/2023 10:47, Konrad Dybcio wrote:
>>>>> On 13.09.2023 09:07, Krzysztof Kozlowski wrote:
>>>>>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>>>>>> These clocks are now handled from within the icc framework and are
>>>>>>
>>>>>> That's a driver behavior, not hardware.
>>>>> I believe we've been over this already..
>>>>>
>>>>> The rationale behind this change is: that hardware, which falls
>>>>> under the "interconnect" class, was previously misrepresented as
>>>>> a bunch of clocks. There are clocks underneath, but accessing them
>>>>> directly would be equivalent to e.g. circumventing the PHY subsystem
>>>>> and initializing your UFS PHY from within the UFS device.
>>>>
>>>> And every time one write such commit msg, how should we remember there
>>>> is some exception and actually it is about clock representation not CCF
>>>> or ICC framework.
>>> So is your reply essentially "fine, but please make it clear in
>>> each commit message"?
>>
>> I am fine with this change. If commit msg had such statement, I would
>> not have doubts :/
> Ok, I'll resend, thanks for confirming!
Is there any one continue working on this?
The bindings already merged while the dtb is not consistent with current
binding files. So dt bindings checks are failed actually.
>
> Konrad
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Thx and BRs,
Aiqun(Maria) Yu