mbox series

[v2,00/14] Clean up RPM bus clocks remnants

Message ID 20230721-topic-rpm_clk_cleanup-v2-0-1e506593b1bd@linaro.org
Headers show
Series Clean up RPM bus clocks remnants | expand

Message

Konrad Dybcio Sept. 12, 2023, 1:31 p.m. UTC
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/20230526-topic-smd_icc-v7-0-09c78c175546@linaro.org/
[2] https://lore.kernel.org/linux-arm-msm/20230721-topic-icc_bindings-v1-0-93e2bc728fb7@linaro.org/

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
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/20230721-topic-rpm_clk_cleanup-v1-0-cf6cd5c621d5@linaro.org

[3] https://lore.kernel.org/linux-arm-msm/20230719073520.2644966-1-alexeymin@postmarketos.org/#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,

Comments

Krzysztof Kozlowski Sept. 13, 2023, 7:15 a.m. UTC | #1
On 12/09/2023 15:31, Konrad Dybcio wrote:
> 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 <konrad.dybcio@linaro.org>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Konrad Dybcio Sept. 13, 2023, 8:47 a.m. UTC | #2
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
>
Krzysztof Kozlowski Sept. 13, 2023, 8:53 a.m. UTC | #3
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
Konrad Dybcio Sept. 13, 2023, 10:48 a.m. UTC | #4
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
Krzysztof Kozlowski Sept. 13, 2023, 11:14 a.m. UTC | #5
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
Konrad Dybcio Sept. 13, 2023, 11:14 a.m. UTC | #6
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
Will Deacon Sept. 18, 2023, 11:05 a.m. UTC | #7
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,
Bjorn Andersson Sept. 20, 2023, 6:30 p.m. UTC | #8
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,
Aiqun Yu (Maria) Jan. 2, 2024, 10:01 a.m. UTC | #9
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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel