2022-12-28 11:45:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

The GCC bindings expect core_bi_pll_test_se clock input, even if it is
optional:

sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index d473194c968d..d5747bb467e0 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -644,6 +644,7 @@ gcc: clock-controller@100000 {
#power-domain-cells = <1>;
clock-names = "bi_tcxo",
"sleep_clk",
+ "core_bi_pll_test_se",
"pcie_0_pipe_clk",
"pcie_1_pipe_clk",
"ufs_card_rx_symbol_0_clk",
@@ -661,6 +662,7 @@ gcc: clock-controller@100000 {
<0>,
<0>,
<0>,
+ <0>,
<&ufs_phy_rx_symbol_0_clk>,
<&ufs_phy_rx_symbol_1_clk>,
<&ufs_phy_tx_symbol_0_clk>,
--
2.34.1


2022-12-28 12:01:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: qcom: sm8350-sony-xperia-sagami: specify which LDO modes are allowed

This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes":

sm8350-sony-xperia-sagami-pdx214.dtb: regulators-0: ldo5: 'regulator-allowed-modes' is a dependency of 'regulator-allow-set-load'

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
index 4862fd69413e..a403fabc8da7 100644
--- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
@@ -171,6 +171,8 @@ pm8350_l5: ldo5 {
regulator-max-microvolt = <888000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};

pm8350_l6: ldo6 {
@@ -179,6 +181,8 @@ pm8350_l6: ldo6 {
regulator-max-microvolt = <1208000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};

pm8350_l7: ldo7 {
@@ -187,6 +191,8 @@ pm8350_l7: ldo7 {
regulator-max-microvolt = <3008000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};

/* L8 - lcx.lvl (ARC) */
@@ -197,6 +203,8 @@ pm8350_l9: ldo9 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
};

--
2.34.1

2022-12-28 12:03:48

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock



On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
> optional:
>
> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
Is it even going to be used by anybody, or should we just drop
it on the driver side as per usual?

Konrad
> arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index d473194c968d..d5747bb467e0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -644,6 +644,7 @@ gcc: clock-controller@100000 {
> #power-domain-cells = <1>;
> clock-names = "bi_tcxo",
> "sleep_clk",
> + "core_bi_pll_test_se",
> "pcie_0_pipe_clk",
> "pcie_1_pipe_clk",
> "ufs_card_rx_symbol_0_clk",
> @@ -661,6 +662,7 @@ gcc: clock-controller@100000 {
> <0>,
> <0>,
> <0>,
> + <0>,
> <&ufs_phy_rx_symbol_0_clk>,
> <&ufs_phy_rx_symbol_1_clk>,
> <&ufs_phy_tx_symbol_0_clk>,

2022-12-28 12:18:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

On 28/12/2022 12:37, Konrad Dybcio wrote:
>
>
> On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
>> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
>> optional:
>>
>> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
> Is it even going to be used by anybody, or should we just drop
> it on the driver side as per usual?

It's mentioned as possible parent, so there might be users somewhere...
Or you want to say that other binding and DTS users cannot use that clock?

Best regards,
Krzysztof

2022-12-28 12:35:16

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock



On 28.12.2022 12:55, Krzysztof Kozlowski wrote:
> On 28/12/2022 12:37, Konrad Dybcio wrote:
>>
>>
>> On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
>>> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
>>> optional:
>>>
>>> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>>>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>> Is it even going to be used by anybody, or should we just drop
>> it on the driver side as per usual?
>
> It's mentioned as possible parent, so there might be users somewhere...
> Or you want to say that other binding and DTS users cannot use that clock?
There's no driver (even downstream) for a supplier of this clock and it's
(probably) only used for early validation by qcom folks. What we're
interested in, as far as debugging clocks goes, is handled by debugcc [1].

Konrad

[1] https://github.com/andersson/debugcc/
>
> Best regards,
> Krzysztof
>

2022-12-28 13:15:12

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

On 28/12/2022 13:55, Krzysztof Kozlowski wrote:
> On 28/12/2022 12:37, Konrad Dybcio wrote:
>>
>>
>> On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
>>> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
>>> optional:
>>>
>>> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>>>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>> Is it even going to be used by anybody, or should we just drop
>> it on the driver side as per usual?
>
> It's mentioned as possible parent, so there might be users somewhere...
> Or you want to say that other binding and DTS users cannot use that clock?

Yes. In the past few months we have been removing the core_bi_pll_test
from the old clock drivers (and new clock drivers mostly lack them).
Let's remove it from the rest of clock drivers.

>
> Best regards,
> Krzysztof
>

--
With best wishes
Dmitry

2022-12-28 14:45:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

On 28/12/2022 13:50, Dmitry Baryshkov wrote:
> On 28/12/2022 13:55, Krzysztof Kozlowski wrote:
>> On 28/12/2022 12:37, Konrad Dybcio wrote:
>>>
>>>
>>> On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
>>>> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
>>>> optional:
>>>>
>>>> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>> ---
>>> Is it even going to be used by anybody, or should we just drop
>>> it on the driver side as per usual?
>>
>> It's mentioned as possible parent, so there might be users somewhere...
>> Or you want to say that other binding and DTS users cannot use that clock?
>
> Yes. In the past few months we have been removing the core_bi_pll_test
> from the old clock drivers (and new clock drivers mostly lack them).
> Let's remove it from the rest of clock drivers.

If you are going to start doing the same work, please at least share it
upfront.

Best regards,
Krzysztof

2022-12-28 15:11:52

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

On 28/12/2022 16:25, Krzysztof Kozlowski wrote:
> On 28/12/2022 13:50, Dmitry Baryshkov wrote:
>> On 28/12/2022 13:55, Krzysztof Kozlowski wrote:
>>> On 28/12/2022 12:37, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 28.12.2022 12:24, Krzysztof Kozlowski wrote:
>>>>> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
>>>>> optional:
>>>>>
>>>>> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>>> ---
>>>> Is it even going to be used by anybody, or should we just drop
>>>> it on the driver side as per usual?
>>>
>>> It's mentioned as possible parent, so there might be users somewhere...
>>> Or you want to say that other binding and DTS users cannot use that clock?
>>
>> Yes. In the past few months we have been removing the core_bi_pll_test
>> from the old clock drivers (and new clock drivers mostly lack them).
>> Let's remove it from the rest of clock drivers.
>
> If you are going to start doing the same work, please at least share it
> upfront.

Excuse me.

--
With best wishes
Dmitry

2022-12-29 17:38:43

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH 1/2] arm64: dts: qcom: sm8350: add missing core_bi_pll_test_se GCC clock

On Wed, 28 Dec 2022 12:24:55 +0100, Krzysztof Kozlowski wrote:
> The GCC bindings expect core_bi_pll_test_se clock input, even if it is
> optional:
>
> sm8350-mtp.dtb: clock-controller@100000: clock-names:2: 'core_bi_pll_test_se' was expected
>
>

Applied, thanks!

[2/2] arm64: dts: qcom: sm8350-sony-xperia-sagami: specify which LDO modes are allowed
commit: 8ea261588fe98d171fcecf477a9f27aea8a06fd0

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