2023-11-21 14:32:50

by Kathiravan Thirumoorthy

[permalink] [raw]
Subject: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition

In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.

So let's drop the binding definition as well.

Signed-off-by: Kathiravan Thirumoorthy <[email protected]>
---
Changes in V2:
- No changes
---
include/dt-bindings/clock/qcom,ipq5332-gcc.h | 3 ---
1 file changed, 3 deletions(-)

diff --git a/include/dt-bindings/clock/qcom,ipq5332-gcc.h b/include/dt-bindings/clock/qcom,ipq5332-gcc.h
index 8a405a0a96d0..4649026da332 100644
--- a/include/dt-bindings/clock/qcom,ipq5332-gcc.h
+++ b/include/dt-bindings/clock/qcom,ipq5332-gcc.h
@@ -55,7 +55,6 @@
#define GCC_NSSCC_CLK 46
#define GCC_NSSCFG_CLK 47
#define GCC_NSSNOC_ATB_CLK 48
-#define GCC_NSSNOC_NSSCC_CLK 49
#define GCC_NSSNOC_QOSGEN_REF_CLK 50
#define GCC_NSSNOC_SNOC_1_CLK 51
#define GCC_NSSNOC_SNOC_CLK 52
@@ -124,8 +123,6 @@
#define GCC_SDCC1_APPS_CLK_SRC 115
#define GCC_SLEEP_CLK_SRC 116
#define GCC_SNOC_LPASS_CFG_CLK 117
-#define GCC_SNOC_NSSNOC_1_CLK 118
-#define GCC_SNOC_NSSNOC_CLK 119
#define GCC_SNOC_PCIE3_1LANE_1_M_CLK 120
#define GCC_SNOC_PCIE3_1LANE_1_S_CLK 121
#define GCC_SNOC_PCIE3_1LANE_M_CLK 122

--
2.34.1


2023-11-21 15:08:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition

On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in

Where is this commit coming from?

> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.

Implementation can change and for example bring back these clocks. Are
you going to change bindings? No, drop the patch.

Bindings should be dropped only in a few rare cases like clocks not
available for OS or bugs.

Best regards,
Krzysztof

2023-11-22 10:09:50

by Kathiravan Thirumoorthy

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition



On 11/21/2023 8:36 PM, Krzysztof Kozlowski wrote:
> On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
>> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
>
> Where is this commit coming from?
>
>> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
>> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.
>
> Implementation can change and for example bring back these clocks. Are
> you going to change bindings? No, drop the patch.
>
> Bindings should be dropped only in a few rare cases like clocks not
> available for OS or bugs.

Thanks Krzysztof. Will drop this patch in V3.

One more question to understand further. In IPQ SoCs there are bunch of
coresight / QDSS clocks but coresight framework doesn't handle all
clocks. Those clocks are enabled in bootloader stage itself. In such
case, should I drop the clocks from both binding and driver or only from
driver?

Thanks,
Kathiravan.

>
> Best regards,
> Krzysztof
>

2023-11-22 10:13:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition

On 22/11/2023 11:08, Kathiravan Thirumoorthy wrote:
>
>
> On 11/21/2023 8:36 PM, Krzysztof Kozlowski wrote:
>> On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
>>> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
>>
>> Where is this commit coming from?
>>
>>> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
>>> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.
>>
>> Implementation can change and for example bring back these clocks. Are
>> you going to change bindings? No, drop the patch.
>>
>> Bindings should be dropped only in a few rare cases like clocks not
>> available for OS or bugs.
>
> Thanks Krzysztof. Will drop this patch in V3.
>
> One more question to understand further. In IPQ SoCs there are bunch of
> coresight / QDSS clocks but coresight framework doesn't handle all
> clocks. Those clocks are enabled in bootloader stage itself. In such
> case, should I drop the clocks from both binding and driver or only from
> driver?

That's not really the reason to drop them at all. Neither from driver,
nor from bindings. You should not rely on bootloader handling your clocks

Best regards,
Krzysztof

2023-11-22 10:19:34

by Kathiravan Thirumoorthy

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition



On 11/22/2023 3:42 PM, Krzysztof Kozlowski wrote:
> On 22/11/2023 11:08, Kathiravan Thirumoorthy wrote:
>>
>>
>> On 11/21/2023 8:36 PM, Krzysztof Kozlowski wrote:
>>> On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
>>>> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
>>>
>>> Where is this commit coming from?
>>>
>>>> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
>>>> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.
>>>
>>> Implementation can change and for example bring back these clocks. Are
>>> you going to change bindings? No, drop the patch.
>>>
>>> Bindings should be dropped only in a few rare cases like clocks not
>>> available for OS or bugs.
>>
>> Thanks Krzysztof. Will drop this patch in V3.
>>
>> One more question to understand further. In IPQ SoCs there are bunch of
>> coresight / QDSS clocks but coresight framework doesn't handle all
>> clocks. Those clocks are enabled in bootloader stage itself. In such
>> case, should I drop the clocks from both binding and driver or only from
>> driver?
>
> That's not really the reason to drop them at all. Neither from driver,
> nor from bindings. You should not rely on bootloader handling your clocks


Thanks, lets say if those clocks are not needed at all by OS since QDSS
is not used and needed only for the boot loaders to access the
corresponding address space, in such case what can be done? I
understand, at first those clocks should not have been added to the driver.


>
> Best regards,
> Krzysztof
>

2023-11-22 10:26:46

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition

On Wed, 22 Nov 2023 at 12:19, Kathiravan Thirumoorthy
<[email protected]> wrote:
>
>
>
> On 11/22/2023 3:42 PM, Krzysztof Kozlowski wrote:
> > On 22/11/2023 11:08, Kathiravan Thirumoorthy wrote:
> >>
> >>
> >> On 11/21/2023 8:36 PM, Krzysztof Kozlowski wrote:
> >>> On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
> >>>> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
> >>>
> >>> Where is this commit coming from?
> >>>
> >>>> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
> >>>> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.
> >>>
> >>> Implementation can change and for example bring back these clocks. Are
> >>> you going to change bindings? No, drop the patch.
> >>>
> >>> Bindings should be dropped only in a few rare cases like clocks not
> >>> available for OS or bugs.
> >>
> >> Thanks Krzysztof. Will drop this patch in V3.
> >>
> >> One more question to understand further. In IPQ SoCs there are bunch of
> >> coresight / QDSS clocks but coresight framework doesn't handle all
> >> clocks. Those clocks are enabled in bootloader stage itself. In such
> >> case, should I drop the clocks from both binding and driver or only from
> >> driver?
> >
> > That's not really the reason to drop them at all. Neither from driver,
> > nor from bindings. You should not rely on bootloader handling your clocks
>
>
> Thanks, lets say if those clocks are not needed at all by OS since QDSS
> is not used and needed only for the boot loaders to access the
> corresponding address space, in such case what can be done? I
> understand, at first those clocks should not have been added to the driver.

First, what is QDSS? Yet another acronym?

Second, if they are not used now, they can get used later.

--
With best wishes
Dmitry

2023-11-22 10:48:01

by Kathiravan Thirumoorthy

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] dt-bindings: clock: ipq5332: drop the few nss clocks definition



On 11/22/2023 3:53 PM, Dmitry Baryshkov wrote:
> On Wed, 22 Nov 2023 at 12:19, Kathiravan Thirumoorthy
> <[email protected]> wrote:
>>
>>
>>
>> On 11/22/2023 3:42 PM, Krzysztof Kozlowski wrote:
>>> On 22/11/2023 11:08, Kathiravan Thirumoorthy wrote:
>>>>
>>>>
>>>> On 11/21/2023 8:36 PM, Krzysztof Kozlowski wrote:
>>>>> On 21/11/2023 15:30, Kathiravan Thirumoorthy wrote:
>>>>>> In commit 0dd3f263c810 ("clk: qcom: ipq5332: enable few nssnoc clocks in
>>>>>
>>>>> Where is this commit coming from?
>>>>>
>>>>>> driver probe"), gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk,
>>>>>> gcc_nssnoc_nsscc_clk are enabled in driver probe to keep it always-on.
>>>>>
>>>>> Implementation can change and for example bring back these clocks. Are
>>>>> you going to change bindings? No, drop the patch.
>>>>>
>>>>> Bindings should be dropped only in a few rare cases like clocks not
>>>>> available for OS or bugs.
>>>>
>>>> Thanks Krzysztof. Will drop this patch in V3.
>>>>
>>>> One more question to understand further. In IPQ SoCs there are bunch of
>>>> coresight / QDSS clocks but coresight framework doesn't handle all
>>>> clocks. Those clocks are enabled in bootloader stage itself. In such
>>>> case, should I drop the clocks from both binding and driver or only from
>>>> driver?
>>>
>>> That's not really the reason to drop them at all. Neither from driver,
>>> nor from bindings. You should not rely on bootloader handling your clocks
>>
>>
>> Thanks, lets say if those clocks are not needed at all by OS since QDSS
>> is not used and needed only for the boot loaders to access the
>> corresponding address space, in such case what can be done? I
>> understand, at first those clocks should not have been added to the driver.
>
> First, what is QDSS? Yet another acronym?

Qualcomm Debug Sub System - which compromises of various debug infra
like coresight, DCC and so on.

>
> Second, if they are not used now, they can get used later.
>

Thanks. I will drop it from driver and leave the bindings as it is.

Thanks,
Kathiravan