Linux clock framework development
 help / color / mirror / Atom feed
* Re: [PATCH 7/7] clk: sunxi-ng: Add Allwinner A733 RTC CCU support
From: Jerome Brunet @ 2026-06-15 17:56 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Junhui Liu, Michael Turquette, Stephen Boyd, Jernej Skrabec,
	Samuel Holland, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Maxime Ripard, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel, linux-rtc,
	devicetree, André Przywara
In-Reply-To: <CAGb2v64euL+QNXiJdTn0JygYLXg0WoguPSprKT4sKGZGVZbwug@mail.gmail.com>

On sam. 28 mars 2026 at 22:41, Chen-Yu Tsai <wens@kernel.org> wrote:

> On Wed, Jan 21, 2026 at 7:04 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>>
>> Add support for the internal CCU found in the RTC module of the Allwinner
>> A733 SoC. While the basic 16MHz (IOSC) and 32kHz logic remains compatible
>> with older SoCs like the sun6i, the A733 introduces several new features.
>>
>> The A733 RTC CCU supports choosing one of three external crystal
>> frequencies: 19.2MHz, 24MHz, and 26MHz. It features hardware detection
>> logic to automatically identify the frequency used on the board and
>> exports this DCXO signal as the "hosc" clock.
>>
>> Furthermore, the driver implements logic to derive a 32kHz reference
>> from the HOSC. This is achieved through a muxed clock path using fixed
>> pre-dividers to normalize the different crystal frequencies to ~32kHz.
>
> Have you tested whether the actually normalizes the frequency, i.e.
> selects a different divider based on the DCXO frequency? Otherwise
> we're just lying about the frequency.
>
>> This path reuses the same hardware mux registers as the HOSC clock.
>>
>> Additionally, this CCU provides several gate clocks for specific
>> peripherals, including SerDes, HDMI, and UFS. The driver is implemented
>> as an auxiliary driver to be bound to the sun6i-rtc driver.
>>
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---

[...]

>> +};
>> +
>> +static const struct clk_parent_data hosc_parents[] = {
>> +       { .fw_name = "osc24M" },
>> +       { .fw_name = "osc19M" },
>> +       { .fw_name = "osc26M" },
>> +       { .fw_name = "osc24M" },
>> +};
>
> As mentioned in my reply to the binding, this is wrong. There is only
> one input.
>
> The most you can do is check the rate of the parent clock against the
> detected one, and _scream_ that the DT is wrong. And maybe override
> the reported frequency.
>
> If you want to do the latter, you could add a new fixed rate gated
> clock type to our library. You would fill in the rate before the
> clocks get registered. I probably wouldn't go that far. We want people
> to have correct hardware descriptions.
>
> Funnily enough Allwinner's BSP actually implements a fixed rate gate
> for the next 24M-to-32k divider clock.

What about implementing the register bellow as a read-only (and
non-cached) divider using the factors provided by Junhui ? That would be
an accurate description of the HW I think.

The oscillator gets set in DT and if the output reported past the
divider is not 32728Hz, you know you've got a problem (bad DT or HW gone
bad)

With a fixed-rate gate, you may actually end up lying about what
actually happen, if the HW does not behave as expected.

Do you prefer a fixed-rate gate still or should I try the RO divider
approach ?

>
>> +
>> +struct ccu_mux hosc_clk = {
>> +       .enable = DCXO_CTRL_DCXO_EN,
>> +       .mux    = _SUNXI_CCU_MUX(14, 2),
>> +       .common = {
>> +               .reg            = DCXO_CTRL_REG,
>> +               .hw.init        = CLK_HW_INIT_PARENTS_DATA("hosc",
>> +                                                          hosc_parents,
>> +                                                          &ccu_mux_ro_ops,
>> +                                                          0),
>> +       },
>> +};
>
> So this is wrong.
>
>> +
>> +static const struct ccu_mux_fixed_prediv hosc_32k_predivs[] = {
>> +       { .index = 0, .div = 732 },
>
> Why is it 732 instead of 750?
>
>> +       { .index = 1, .div = 586 },
>> +       { .index = 2, .div = 793 },
>> +       { .index = 3, .div = 732 },
>> +};
>> +
>> +static struct ccu_mux hosc_32k_mux_clk = {
>> +       .enable         = DCXO_CTRL_DCXO_EN,
>

^ permalink raw reply

* Re: [PATCH 1/7] dt-bindings: rtc: sun6i: Add Allwinner A733 support
From: Jerome Brunet @ 2026-06-15 17:46 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Junhui Liu, Michael Turquette, Stephen Boyd, Jernej Skrabec,
	Samuel Holland, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Maxime Ripard, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel, linux-rtc,
	devicetree
In-Reply-To: <CAGb2v67844OPwE6VJ0PAs5LsmCa2h0FvXOBUomZ50dM5tZ0Zow@mail.gmail.com>

On sam. 28 mars 2026 at 20:37, Chen-Yu Tsai <wens@kernel.org> wrote:

> On Wed, Jan 21, 2026 at 7:03 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>>
>> The RTC module in the Allwinner A733 SoC is functionally compatible with
>> the sun6i RTC, but its internal Clock Control Unit (CCU) has significant
>> changes.
>>
>> The A733 supports selecting the oscillator between three frequencies:
>> 19.2MHz, 24MHz, and 26MHz. The RTC CCU relies on hardware to detect
>> which frequency is actually used on the board. By defining all three
>> frequencies as fixed-clocks in the device tree, the driver can identify
>> the hardware-detected frequency and expose it to the rest of the system.
>
> No. The board device tree shall have the exact and correct frequency
> defined in the external crystal device node. The operating system can
> use the hardware-detected frequency to "fix" the in-system representation
> if it is off.
>
>> Additionally, the A733 RTC CCU provides several new DCXO gate clocks for
>> specific modules, including SerDes, HDMI, and UFS.
>>
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---
>>  .../bindings/rtc/allwinner,sun6i-a31-rtc.yaml      | 38 ++++++++++++++++++++--
>>  include/dt-bindings/clock/sun60i-a733-rtc.h        | 16 +++++++++
>>  2 files changed, 52 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
>> index 9df5cdb6f63f..b18431955783 100644
>> --- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
>> +++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
>> @@ -26,6 +26,7 @@ properties:
>>            - allwinner,sun50i-h6-rtc
>>            - allwinner,sun50i-h616-rtc
>>            - allwinner,sun50i-r329-rtc
>> +          - allwinner,sun60i-a733-rtc
>>        - items:
>>            - const: allwinner,sun50i-a64-rtc
>>            - const: allwinner,sun8i-h3-rtc
>> @@ -46,11 +47,11 @@ properties:
>>
>>    clocks:
>>      minItems: 1
>> -    maxItems: 4
>> +    maxItems: 6
>>
>>    clock-names:
>>      minItems: 1
>> -    maxItems: 4
>> +    maxItems: 6
>>
>>    clock-output-names:
>>      minItems: 1
>> @@ -156,6 +157,38 @@ allOf:
>>          - clocks
>>          - clock-names
>>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: allwinner,sun60i-a733-rtc
>> +
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 5
>> +          items:
>> +            - description: Bus clock for register access
>
>> +            - description: 19.2 MHz oscillator
>> +            - description: 24 MHz oscillator
>> +            - description: 26 MHz oscillator
>
> No. There is only one input. As in there is only one set of pins for the
> DCXO. The inputs are the same as on R329 / A523. Just use that list.
>
>> +            - description: AHB parent for internal SPI clock
>> +            - description: External 32768 Hz oscillator
>> +
>> +        clock-names:
>> +          minItems: 5
>> +          items:
>> +            - const: bus
>> +            - const: osc19M
>> +            - const: osc24M
>> +            - const: osc26M
>> +            - const: ahb
>> +            - const: ext-osc32k
>> +
>> +      required:
>> +        - clocks
>> +        - clock-names
>> +
>>    - if:
>>        properties:
>>          compatible:
>> @@ -164,6 +197,7 @@ allOf:
>>                - allwinner,sun8i-r40-rtc
>>                - allwinner,sun50i-h616-rtc
>>                - allwinner,sun50i-r329-rtc
>> +              - allwinner,sun60i-a733-rtc
>>
>>      then:
>>        properties:
>> diff --git a/include/dt-bindings/clock/sun60i-a733-rtc.h b/include/dt-bindings/clock/sun60i-a733-rtc.h
>> new file mode 100644
>> index 000000000000..8a2b5facad73
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/sun60i-a733-rtc.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
>> +
>> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_
>> +#define _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_
>> +
>> +#define CLK_IOSC               0
>> +#define CLK_OSC32K             1
>> +#define CLK_HOSC               2
>
> The DCXO enable control has been present since at least the H6. We just
> never added it, as we would never disable it anyway.
>
> If you compare the RTC clock trees of the A733 and A523, the only addition
> besides the new gates seems to be the LOSC auto selection. But even that
> is just an illusion, as the A523 has the same registers for that.
>
> One could say the A733 RTC is almost backward compatible to the A523, if
> not for the two fastboot registers the A523 has at 0x120 and 0x124.
>
> So I ask that you try to integrate the differences into the existing
> driver and bindings. You can tweak and export internal clks if you
> need.

I'd like to help with that. I think it is doable but I have a question
regarding the binding of the existing driver, more precisely their usage
here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c?h=v7.1#n370

Clock indexes are supposed to be stable in DT (AFAIK) but with the code
linked the external 32k is at:

* "ext-32k" - so index 3 - if "clock-names" is present
* index 0 if clock names is not present

... but index 0 is supposed to be the bus clock according the binding
doc, whether "clock-names" is there or not :/

So what are those old r329 bindings ? is there a documentation defining
them somewhere ?

Cleaning that part would help with A733 addition in the existing driver
I think

>
>> +#define CLK_RTC_32K            3
>
> AFAICT besides being an internal clock, this is also fed to GPIO for
> debounce? We probably need to expose this on the A523 as well.
>
>
> Thanks
> ChenYu
>
>
>> +#define CLK_OSC32K_FANOUT      4
>> +#define CLK_HOSC_SERDES1       5
>> +#define CLK_HOSC_SERDES0       6
>> +#define CLK_HOSC_HDMI          7
>> +#define CLK_HOSC_UFS           8
>> +
>> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_ */
>>
>> --
>> 2.52.0
>>
>>

-- 
Jerome

^ permalink raw reply

* Re: [PATCH 1/2] clk: qcom: gcc-msm8660: register CE2 H clock
From: me @ 2026-06-15 17:21 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Herman van Hazendonk, sboyd, Bjorn Andersson, Michael Turquette,
	linux-arm-msm, linux-clk, linux-kernel
In-Reply-To: <b51b4d7e-fdea-4793-9007-c9e3ee922f70@oss.qualcomm.com>

On 2026-06-15 18:36, Konrad Dybcio wrote:
> On 6/2/26 6:27 AM, Herman van Hazendonk wrote:
>> On MSM8x60 the Crypto Engine 2 (CE2) block at 0x18500000 is gated by
>> a single hardware enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The
>> existing dt-binding header already reserves CE2_H_CLK (ID 77) for
>> this clock but the driver never registered an entry for it, so probe
>> of any consumer that resolves the binding fails: the CE2 MMIO window
>> reads back 0x0 and qce's DMA hangs indefinitely waiting for handshake
>> signals that never arrive.
> 
> [...]
> 
>> +/*
>> + * Crypto Engine 2 (CE2) clock.
>> + *
>> + * On MSM8x60 the CE2 block at 0x18500000 is gated by a single 
>> hardware
>> + * enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The vendor MSM8660
>> + * clock-8x60.c routes both the "core" and "iface" consumer-name 
>> lookups
>> + * to this one register, and the upstream QCE crypto driver requests
>> + * both clock names via devm_clk_get_optional_enabled(). Without the
>> + * clock present at probe the QCE MMIO window reads back 0x0 and DMA
>> + * hangs indefinitely waiting for handshake signals that never 
>> arrive.
>> + *
>> + * Register a single clk_branch: the consumer DT can reference the 
>> same
>> + * clock phandle twice under different clock-names ("core" and 
>> "iface"),
>> + * which yields the same struct clk for both clk_get() calls. Per-
>> + * consumer refcounting then works correctly and the single 
>> underlying
>> + * enable bit is asserted while either consumer holds the clock
>> + * prepared, instead of having two independent clk_branch structs
>> + * racing the same hardware bit.
>> + */
> 
> I don't find this comment particularly valuable, given it ends up
> describing the fact that the common clock framework has refcounted
> enables (pretty widely known) and details how the DT is going to use
> this (which we can read in the DT itself)
> 
> I think the commit message is really exhaustive and it's a better
> place for this info anyway
> 
> Konrad
Hi Konrad,

Happy to clean it up. MSM8x60 is poorly documented in public and 
whatever is
available in downstream kernels is often incomplete, so I tried to 
document
most in the various commits. Happy to put it in another place if that's 
more
appropriate. A lot of the info was found by register poking and trial 
and
error because the lack of documentation.

Thanks,
Herman

^ permalink raw reply

* Re: [PATCH] clk: mvebu: ap-cpu: fix missing clk_put() in ap_cpu_clock_probe()
From: Brian Masney @ 2026-06-15 17:18 UTC (permalink / raw)
  To: Wentao Liang
  Cc: andrew, gregory.clement, sebastian.hesselbarth, mturquette, sboyd,
	linux-arm-kernel, linux-clk, linux-kernel, stable
In-Reply-To: <20260604025115.3763823-1-vulab@iscas.ac.cn>

On Thu, Jun 04, 2026 at 02:51:15AM +0000, Wentao Liang wrote:
> The function ap_cpu_clock_probe() calls of_clk_get() to obtain a
> reference to the parent clock for each CPU cluster, but it never
> releases it with clk_put().  The returned clk is used only to read
> the parent's name via __clk_get_name(), and the reference is leaked
> on every successful cluster initialization as well as on the error
> path when devm_clk_hw_register() fails.
> 
> Add the missing clk_put() after the name has been extracted and
> before returning on error to fix the leak.
> 
> Fixes: af9617b419f7 ("clk: mvebu: ap-cpu-clk: Fix a memory leak in error handling paths")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

This calls:

                parent = of_clk_get(np, cluster_index);
                if (IS_ERR(parent)) {
			...
                }
                parent_name =  __clk_get_name(parent);

Can this all be replaced with a call to of_clk_get_parent_name() ?

Brian


^ permalink raw reply

* [PATCH v2 6/6] arm64: dts: qcom: sm8650: Add missing CX power domain to GCC
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong, Konrad Dybcio, Abel Vesa
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

Unless CX is declared as the power-domain of GCC, votes (power and
performance) on the GDSCs it provides will not propagate to the CX,
which might result in under-voltage conditions.

Add the missing power-domains property to associate GCC with RPMHPD_CX.

Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 160ead25ecf7..8365e97436d6 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -1963,6 +1963,8 @@ gcc: clock-controller@100000 {
 				 <&ufs_mem_phy 2>,
 				 <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
 
+			power-domains = <&rpmhpd RPMHPD_CX>;
+
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 5/6] arm64: dts: qcom: sm8550: Add missing CX power domain to GCC
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong, Konrad Dybcio, Abel Vesa
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

Unless CX is declared as the power-domain of GCC, votes (power and
performance) on the GDSCs it provides will not propagate to the CX,
which might result in under-voltage conditions.

Add the missing power-domains property to associate GCC with RPMHPD_CX.

Fixes: ffc50b2d3828 ("arm64: dts: qcom: Add base SM8550 dtsi")
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 396201905ef2..2cadcaae9e01 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -1183,6 +1183,7 @@ soc: soc@0 {
 		gcc: clock-controller@100000 {
 			compatible = "qcom,sm8550-gcc";
 			reg = <0 0x00100000 0 0x1f4200>;
+			power-domains = <&rpmhpd RPMHPD_CX>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 4/6] arm64: dts: qcom: sm8450: Add missing CX power domain to GCC
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong, Abel Vesa, Konrad Dybcio
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

Unless CX is declared as the power-domain of GCC, votes (power and
performance) on the GDSCs it provides will not propagate to the CX,
which might result in under-voltage conditions.

Add the missing power-domains property to associate GCC with RPMHPD_CX.

Fixes: 5188049c9b36 ("arm64: dts: qcom: Add base SM8450 DTSI")
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 56cb6e959e4e..2639790307d8 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -935,6 +935,7 @@ soc: soc@0 {
 		gcc: clock-controller@100000 {
 			compatible = "qcom,gcc-sm8450";
 			reg = <0x0 0x00100000 0x0 0x1f4200>;
+			power-domains = <&rpmhpd RPMHPD_CX>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 3/6] dt-bindings: clock: qcom: sm8650-gcc: Add missing power-domains property
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

In order for the GCC votes on the GDSCs it provides to be propagated
to CX, CX needs to be declared as power domain of the GCC.

Document the missing power-domains property to that purpose.

Fixes: b69d932154dc ("dt-bindings: clock: qcom: document the SM8650 General Clock Controller")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml
index c7143e2abc80..9c1504827cbf 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml
@@ -32,9 +32,14 @@ properties:
       - description: UFS Phy Tx symbol 0 clock source
       - description: USB3 Phy wrapper pipe clock source
 
+  power-domains:
+    items:
+      - description: CX domain
+
 required:
   - compatible
   - clocks
+  - power-domains
   - '#power-domain-cells'
 
 allOf:
@@ -58,6 +63,7 @@ examples:
                <&ufs_mem_phy 1>,
                <&ufs_mem_phy 2>,
                <&usb_1_qmpphy>;
+      power-domains = <&rpmhpd RPMHPD_CX>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/6] dt-bindings: clock: qcom: sm8550-gcc: Add missing power-domains property
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

In order for the GCC votes on the GDSCs it provides to be propagated
to CX, CX needs to be declared as power domain of the GCC.

Document the missing power-domains property to that purpose.

Fixes: 47ba9c50bbeb ("dt-bindings: clock: Add SM8550 GCC clocks")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml
index c4e9b9bb63f5..19f06d27c7c1 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml
@@ -31,9 +31,14 @@ properties:
       - description: UFS Phy Tx symbol 0 clock source
       - description: USB3 Phy wrapper pipe clock source
 
+  power-domains:
+    items:
+      - description: CX domain
+
 required:
   - compatible
   - clocks
+  - power-domains
   - '#power-domain-cells'
 
 allOf:
@@ -55,6 +60,7 @@ examples:
                <&ufs_mem_phy 1>,
                <&ufs_mem_phy 2>,
                <&usb_1_qmpphy>;
+      power-domains = <&rpmhpd RPMHPD_CX>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 1/6] dt-bindings: clock: qcom: gcc-sm8450: Add missing power-domains property
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org>

In order for the GCC votes on the GDSCs it provides to be propagated
to CX, CX needs to be declared as power domain of the GCC.

Document the missing power-domains property to that purpose.

Fixes: 72a0ca203ca7 ("dt-bindings: clock: Add SM8450 GCC clock bindings")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml
index 3169ac05e1d8..7e3713cfd498 100644
--- a/Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml
@@ -47,10 +47,15 @@ properties:
       - const: usb3_phy_wrapper_gcc_usb30_pipe_clk # Optional clock
     minItems: 2
 
+  power-domains:
+    items:
+      - description: CX domain
+
 required:
   - compatible
   - clocks
   - clock-names
+  - power-domains
   - '#power-domain-cells'
 
 allOf:
@@ -66,6 +71,7 @@ examples:
       reg = <0x00100000 0x001f4200>;
       clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>;
       clock-names = "bi_tcxo", "sleep_clk";
+      power-domains = <&rpmhpd RPMHPD_CX>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 0/6] arm64: dts: qcom: sm8[456]50: Add missing CX power domain to GCC
From: Neil Armstrong @ 2026-06-15 16:57 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Abel Vesa,
	Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
  Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel,
	Krzysztof Kozlowski, Neil Armstrong, Abel Vesa, Konrad Dybcio

Recently, on Eliza & Milos, the CX has been tied up to the GCC,
but this is valid for most platforms including sm8[456]50.
So tie the CX power domain to the GCC as well, for the
same exact reasons as on Eliza & Milos.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v2:
- Add missing power domains in example
- Pick review tags
- Link to v1: https://patch.msgid.link/20260424-topic-sm8x50-tie-gcc-to-cx-v1-0-4b6e09d532ce@linaro.org

---
Neil Armstrong (6):
      dt-bindings: clock: qcom: gcc-sm8450: Add missing power-domains property
      dt-bindings: clock: qcom: sm8550-gcc: Add missing power-domains property
      dt-bindings: clock: qcom: sm8650-gcc: Add missing power-domains property
      arm64: dts: qcom: sm8450: Add missing CX power domain to GCC
      arm64: dts: qcom: sm8550: Add missing CX power domain to GCC
      arm64: dts: qcom: sm8650: Add missing CX power domain to GCC

 Documentation/devicetree/bindings/clock/qcom,gcc-sm8450.yaml | 6 ++++++
 Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml | 6 ++++++
 Documentation/devicetree/bindings/clock/qcom,sm8650-gcc.yaml | 6 ++++++
 arch/arm64/boot/dts/qcom/sm8450.dtsi                         | 1 +
 arch/arm64/boot/dts/qcom/sm8550.dtsi                         | 1 +
 arch/arm64/boot/dts/qcom/sm8650.dtsi                         | 2 ++
 6 files changed, 22 insertions(+)
---
base-commit: 8d6dbbbe3ba62de0a63e962ee004afb848c8e3ac
change-id: 20260424-topic-sm8x50-tie-gcc-to-cx-e756afa72bb8

Best regards,
--  
Neil Armstrong <neil.armstrong@linaro.org>


^ permalink raw reply

* Re: [PATCH 3/8] dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for static-config
From: Conor Dooley @ 2026-06-15 16:44 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-clk, linux-kernel, git, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	devicetree, linux-arm-kernel
In-Reply-To: <20260615-squid-showy-435c9cf780a0@spud>

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Mon, Jun 15, 2026 at 05:42:17PM +0100, Conor Dooley wrote:
> 
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> pw-bot: not-applicable

Actually, I take this back. Patch 1 seems to be what's adding the static
configurations in the first place and then patches 2 and 3 complete that
effort. Instead, please add this static config support as one patch.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH] clk: at91: Read "reg" with helper
From: Brian Masney @ 2026-06-15 16:42 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Michael Turquette, Stephen Boyd, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, linux-clk, linux-arm-kernel, linux-kernel
In-Reply-To: <20260612215251.1888345-1-robh@kernel.org>

On Fri, Jun 12, 2026 at 04:52:51PM -0500, Rob Herring (Arm) wrote:
> The "reg" property is an address-sized DT cell property. The AT91
> compat clock parser only uses a small bus id from it, but reading it
> with the u8 helper does not match the property encoding.
> 
> Use of_property_read_reg() so the code goes through the helper for
> "reg" properties, then keep the existing range check before passing
> the bus id to the clock registration code.
> 
> Assisted-by: Codex:gpt-5-5
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply

* Re: [PATCH 3/8] dt-bindings: clock: clocking-wizard: Make s_axi_aclk optional for static-config
From: Conor Dooley @ 2026-06-15 16:42 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-clk, linux-kernel, git, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michal Simek,
	devicetree, linux-arm-kernel
In-Reply-To: <20260615034845.3320286-4-shubhrajyoti.datta@amd.com>

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]



Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs
From: Brian Masney @ 2026-06-15 16:40 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sudeep Holla, Cristian Marussi, Sebin Francis,
	linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
	Peng Fan
In-Reply-To: <20260612-clk-v10-v10-4-eb92484eda38@nxp.com>

On Fri, Jun 12, 2026 at 04:46:26PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add KUnit test coverage for the assigned-clock-sscs DT property that
> configures spread spectrum on clocks before they are used.
> 
> Extend the existing test infrastructure to support spread spectrum:
> - Add struct clk_spread_spectrum field to clk_dummy_context and a
>   clk_dummy_set_spread_spectrum callback
> - Wire set_spread_spectrum into all dummy clock ops
> - Extend clk_assigned_rates_register_clk and test parameter struct
>   to propagate initial SSCS values
> 
> Add a new separate test suite clk_assigned_sscs with three categories:
> 
>   1. clk_assigned_sscs_assigns_one — verifies that a single
>      assigned-clock-sscs entry correctly configures spread spectrum
>      on one clock, testing both provider and consumer paths
> 
>   2. clk_assigned_sscs_assigns_multiple — verifies that multiple
>      assigned-clock-sscs entries configure spread spectrum on two
>      clocks, testing both provider and consumer paths
> 
>   3. clk_assigned_sscs_skips — verifies that malformed DT properties
>      are correctly skipped without error: missing assigned-clocks,
>      zero-valued SSCS, and null phandles, tested for both provider
>      and consumer scenarios
> 
> New DT overlays are added for all test scenarios:
>   - kunit_clk_assigned_sscs_one{,consumer} — single valid entry
>   - kunit_clk_assigned_sscs_multiple{,consumer} — two valid entries
>   - kunit_clk_assigned_sscs_without{,consumer} — missing assigned-clocks
>   - kunit_clk_assigned_sscs_zero{,consumer} — all-zero SSCS values
>   - kunit_clk_assigned_sscs_null{,consumer} — null phandle
> 
> Co-developed-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Looks good to me.

It's probably not appropriate for me to also put a Reviewed-by here.

Brian


^ permalink raw reply

* Re: [PATCH 1/2] clk: qcom: gcc-msm8660: register CE2 H clock
From: Konrad Dybcio @ 2026-06-15 16:36 UTC (permalink / raw)
  To: Herman van Hazendonk, sboyd
  Cc: Bjorn Andersson, Michael Turquette, linux-arm-msm, linux-clk,
	linux-kernel
In-Reply-To: <20260602042747.277270-2-github.com@herrie.org>

On 6/2/26 6:27 AM, Herman van Hazendonk wrote:
> On MSM8x60 the Crypto Engine 2 (CE2) block at 0x18500000 is gated by
> a single hardware enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The
> existing dt-binding header already reserves CE2_H_CLK (ID 77) for
> this clock but the driver never registered an entry for it, so probe
> of any consumer that resolves the binding fails: the CE2 MMIO window
> reads back 0x0 and qce's DMA hangs indefinitely waiting for handshake
> signals that never arrive.

[...]

> +/*
> + * Crypto Engine 2 (CE2) clock.
> + *
> + * On MSM8x60 the CE2 block at 0x18500000 is gated by a single hardware
> + * enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The vendor MSM8660
> + * clock-8x60.c routes both the "core" and "iface" consumer-name lookups
> + * to this one register, and the upstream QCE crypto driver requests
> + * both clock names via devm_clk_get_optional_enabled(). Without the
> + * clock present at probe the QCE MMIO window reads back 0x0 and DMA
> + * hangs indefinitely waiting for handshake signals that never arrive.
> + *
> + * Register a single clk_branch: the consumer DT can reference the same
> + * clock phandle twice under different clock-names ("core" and "iface"),
> + * which yields the same struct clk for both clk_get() calls. Per-
> + * consumer refcounting then works correctly and the single underlying
> + * enable bit is asserted while either consumer holds the clock
> + * prepared, instead of having two independent clk_branch structs
> + * racing the same hardware bit.
> + */

I don't find this comment particularly valuable, given it ends up
describing the fact that the common clock framework has refcounted
enables (pretty widely known) and details how the DT is going to use
this (which we can read in the DT itself)

I think the commit message is really exhaustive and it's a better
place for this info anyway

Konrad

^ permalink raw reply

* Re: [PATCH next] drivers/clk/clk_test: Use strscpy() to copy the test description
From: Brian Masney @ 2026-06-15 16:33 UTC (permalink / raw)
  To: david.laight.linux
  Cc: Kees Cook, linux-hardening, Arnd Bergmann, linux-clk,
	linux-kernel, Michael Turquette, Stephen Boyd
In-Reply-To: <20260606202633.5018-35-david.laight.linux@gmail.com>

On Sat, Jun 06, 2026 at 09:26:29PM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> The buffer length is known to be KUNIT_PARAM_DESC_SIZE
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply

* Re: [PATCH] clk: clocking-wizard: fix integer overflow in rate calculation
From: Brian Masney @ 2026-06-15 16:32 UTC (permalink / raw)
  To: Pavel Löbl; +Cc: Stephen Boyd, Michal Simek, linux-clk, stable
In-Reply-To: <20260605130340.3549582-1-pavel@loebl.cz>

On Fri, Jun 05, 2026 at 03:03:40PM +0200, Pavel Löbl wrote:
> When using driver on Zynq-7000 (32-bit) determine_rate calculation
> overflows. For instance requesting 32MHz with 100MHz parent clock
> results in 100000000*(4*1000+0) 32-bit multiplication.
> 
> Replace the expression with mult_frac which is already used in
> clk_wzrd_recalc_ratef.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7681f64e6404 ("clk: clocking-wizard: calculate dividers fractional parts")
> Signed-off-by: Pale Löbl <pavel@loebl.cz>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply

* Re: [PATCH] clk: eswin: Add CLK_IGNORE_UNUSED to NoC clock
From: Brian Masney @ 2026-06-15 16:25 UTC (permalink / raw)
  To: dongxuyang
  Cc: mturquette, sboyd, linux-clk, linux-kernel, huangyifeng, ningyu,
	linmin, pinkesh.vaghela
In-Reply-To: <20260605092118.1945-1-dongxuyang@eswincomputing.com>

On Fri, Jun 05, 2026 at 05:21:18PM +0800, dongxuyang@eswincomputing.com wrote:
> From: Xuyang Dong <dongxuyang@eswincomputing.com>
> 
> The gate_noc_nsp_clk provides the essential clock source for NPU,
> DSP, and PCIe subsystems. During kernel init, the clock framework
> attempts to disable unused clocks when clk_ignore_unused kernel
> parameter is not set.
> However, gate_noc_nsp_clk is required to remain enabled for these
> critical subsystems to function properly, causing PCIe boot failures
> when auto-disabled.
> 
> Add CLK_IGNORE_UNUSED flag to gate_noc_nsp_clk to ensure it stays
> enabled even when clk_ignore_unused is not specified in kernel
> command line.
> 
> Fixes: cd44f127c1d4 ("clk: eswin: Add eic7700 clock driver")
> Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>

FYI, I posted a series to implement sync_state support for the clk
subsystem, which should fix these type of issues for the long term. If
you happen to have time, I would appreciate it if you have time to test
this series on your hardware, and drop a Tested-by.

https://lore.kernel.org/linux-clk/20260603-clk-sync-state-v1-0-457120eed200@redhat.com/

I have a new series ready that I'm planning to post tomorrow. I'm
waiting to hear back about the ongoing conversation on that thread.

Once this lands, there shouldn't be much need for the CLK_IGNORE_UNUSED
flag. I'm curious if you revert this patch in your tree if the need for the
flag goes away with the sync_state changes applied.

Let's get your change in the tree though. If we merged sync_state today,
it won't be eligible to be backported to the stable kernels, and the
CLK_IGNORE_UNUSED flag is an easier backport.

Brian


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: clock: renesas,r9a09g077/87: Add PCLKRTC clock ID
From: Conor Dooley @ 2026-06-15 16:22 UTC (permalink / raw)
  To: Prabhakar
  Cc: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Brian Masney,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm,
	linux-kernel, linux-renesas-soc, linux-clk, devicetree, Biju Das,
	Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615143943.1610095-2-prabhakar.mahadev-lad.rj@bp.renesas.com>

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH V2] clk: tegra: Support unique names for multi-socket platforms
From: Brian Masney @ 2026-06-15 16:08 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Michael Turquette, Stephen Boyd, Thierry Reding, linux-clk,
	linux-tegra, Timo Alho
In-Reply-To: <20260602084334.350895-1-jonathanh@nvidia.com>

On Tue, Jun 02, 2026 at 09:43:34AM +0100, Jon Hunter wrote:
> On multi-socket platforms each socket has its own BPMP which exposes the
> same clock names. Fix this by using the NUMA ID as a prefix for the
> clock names on multi-socket platforms.
> 
> Use 'sizeof(info->name)' in the strscpy() and snprintf() functions to
> future proof against anyone changing the size of the 'name' array.
> 
> Co-developed-by: Timo Alho <talho@nvidia.com>
> Signed-off-by: Timo Alho <talho@nvidia.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply

* Re: [PATCH] clk: zynq: handle kasprintf() failure in periph_clk registration
From: Brian Masney @ 2026-06-15 16:07 UTC (permalink / raw)
  To: William Theesfeld
  Cc: Michael Turquette, Stephen Boyd, Michal Simek, linux-clk,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260601203500.658135-1-william@theesfeld.net>

On Mon, Jun 01, 2026 at 04:35:00PM -0400, William Theesfeld wrote:
> zynq_clk_register_periph_clk() ignores the return value of the two
> kasprintf() calls used to build the mux and divider clock names, and
> passes the resulting (possibly NULL) pointers straight into
> clk_register_mux(), clk_register_divider() and clk_register_gate() as
> the clock '"'name'"' argument.  On allocation failure that name later
> gets dereferenced by the clock framework (e.g. in debugfs name
> formatting), causing a NULL-pointer dereference.
> 
> Check both kasprintf() returns.  On failure unwind any allocated name
> buffer and the spinlock, then fall through to the existing err label
> which sets clks[] to ERR_PTR(-ENOMEM).  Freeing the spinlock on the
> error path is correct here because no clk_register_*() call has had
> a chance to take ownership of it; the success path intentionally
> hands it off to the registered clocks.
> 
> The neighbouring zynq_clk_register_fclk() in the same file already
> uses this per-allocation goto-label cleanup pattern; this change
> brings periph_clk into line with it.
> 
> Signed-off-by: William Theesfeld <william@theesfeld.net>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply

* Re: [PATCH v2 7/8] dt-bindings: display: allwinner: Split H616 DE33 layer reg space
From: Jernej Škrabec @ 2026-06-15 15:47 UTC (permalink / raw)
  To: wens, Krzysztof Kozlowski
  Cc: samuel, mripard, maarten.lankhorst, tzimmermann, airlied, simona,
	robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel, linux-clk
In-Reply-To: <86943057-f5b4-4fae-9172-45f13814494f@kernel.org>

Dne ponedeljek, 15. junij 2026 ob 06:28:54 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
> On 14/06/2026 16:08, Jernej Škrabec wrote:
> > Dne ponedeljek, 25. maj 2026 ob 14:10:38 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
> >> On 24/05/2026 23:33, Chen-Yu Tsai wrote:
> >>> Hi,
> >>>
> >>> (resent from new email)
> >>>
> >>> On Thu, May 14, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>
> >>>> On Sat, May 09, 2026 at 09:00:14PM +0200, Jernej Skrabec wrote:
> >>>>> From: Jernej Skrabec <jernej.skrabec@gmail.com>
> >>>>>
> >>>>> As it turns out, current H616 DE33 binding was written based on
> >>>>> incomplete understanding of DE33 design. Namely, planes are shared
> >>>>> resource and not tied to specific mixer, which was the case for previous
> >>>>> generations of Display Engine (DE3 and earlier).
> >>>>>
> >>>>> This means that current DE33 binding doesn't properly reflect HW and
> >>>>> using it would mean that second mixer (used for second display output)
> >>>>> can't be supported.
> >>>>>
> >>>>> Remove layer register space, which will be represented with additional
> >>>>> node, and replace it with phandle, which will point to that new, shared
> >>>>> node. That way, all mixers can share same layers.
> >>>>>
> >>>>> There is no user of this binding yet, so changes can be made safely,
> >>>>> without breaking any backward compatibility.
> >>>>
> >>>> There is user. git grep gives me:
> >>>> drivers/gpu/drm/sun4i/sun8i_mixer.c
> >>>>
> >>>> which means this is a released ABI. As I understood, the old code was
> >>>
> >>> We held off on merging the DT changes so that we could rework this.
> >>> I can't find the actual request though. It was probably over IRC.
> >>>
> >>>> working fine but just did not support all use cases. Why this cannot be
> >>>> kept backwards compatible?
> >>>
> >>> AFAIK the "planes" block is shared between two display mixers. As the
> >>> commit message explains, this prevents using the second mixer, since
> >>> only one of them can claim and map the register space. And on the H700
> >>> (which is the same die as the H616 discussed here but with more exposed
> >>> interfaces), there could actually be a use case for the second mixer.
> >>
> >> It explains why you want to make the changes but not why you cannot keep
> >> it backwards compatible.
> > 
> > I guess it can be backward compatible, but I don't think it makes sense.
> > Yes, original driver implemented original DT bindings, but there is no node
> > which uses that binding. If there is no user of that, why would driver
> 
> Did you check all out of tree users of the ABI? All vendor kernels,
> forks and all of them for which the ABI was made for?

Since when do we care about out of tree users? I understand that drivers
must support old device tree files. Once they work, compatibility must
be carried forward. But that's not the case here.

In any case, vendor kernels have completely different DT structure. This
was developed independently from them. Take a look at [1] how BSP DT looks
like, specifically Display Engine node.

Of course there are some distros which grab WIP patches from mailing lists
soon after they are available. For example, I know that Armbian carried old
WIP patches which used old ABI. However, such distros generally don't care
about exact solution and ditch patches as soon as proper solution is merged
upstream or even when better WIP patches come around. DT files in such
distros get updated alongside kernel, they are not hidden in firmware. 

Best regards,
Jernej

[1] https://github.com/orangepi-xunlong/linux-orangepi/blob/orange-pi-4.9-sun50iw9/arch/arm64/boot/dts/sunxi/sun50iw9p1.dtsi#L1315-L1339

> 
> If there is no single downstream/out of tree kernel using this ABI, then
> of course you do not need to consider it. I don't know how would you
> prove that but I am open for suggestions.
> 
> > need to support it nevertheless? Supporting only actually used DT binding
> > allows for better code architecture, as there is no need to support second,
> > unused path. It also simplifies testing, since developer doesn't need to
> > test both paths if code is changed in that area.
> > 
> Best regards,
> Krzysztof
> 





^ permalink raw reply

* Re: [PATCH 2/4] clk: qcom: common: introduce qcom_cc_sync_state()
From: Brian Masney @ 2026-06-15 15:04 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Dmitry Baryshkov, Val Packett, Saravana Kannan, Abel Vesa,
	Maxime Ripard, Michael Turquette, Stephen Boyd, Russell King,
	Bjorn Andersson, Hans de Goede, linux-clk, linux-kernel,
	linux-arm-msm
In-Reply-To: <fef227d8-4ad0-4884-98ae-fc48056c3856@oss.qualcomm.com>

Hi Konrad / Dmitry,

On Mon, Jun 15, 2026 at 04:51:35PM +0200, Konrad Dybcio wrote:
> On 6/15/26 4:48 PM, Brian Masney wrote:
> > On Mon, Jun 15, 2026 at 04:33:17PM +0200, Konrad Dybcio wrote:
> >> On 6/15/26 4:24 PM, Brian Masney wrote:
> >>> On Sun, Jun 07, 2026 at 01:30:03PM +0300, Dmitry Baryshkov wrote:
> >>>> On Sun, Jun 07, 2026 at 01:43:06AM -0300, Val Packett wrote:
> >>>>>
> >>>>> On 6/6/26 8:15 AM, Dmitry Baryshkov wrote:
> >>>>>> On Wed, Jun 03, 2026 at 10:21:47AM -0400, Brian Masney wrote:
> >>>>>>> Several qcom clk providers currently have a sync_state helper set to
> >>>>>>> icc_sync_state(). With an upcoming change to the clk framework, if
> >>>>>>> sync_state is not defined for the device, then the clk framework sets it
> >>>>>>> to clk_sync_state().
> >>>>>>> [..]
> >>>>>>> @@ -464,5 +466,12 @@ int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
> >>>>>>>   }
> >>>>>>>   EXPORT_SYMBOL_GPL(qcom_cc_probe_by_index);
> >>>>>>> +void qcom_cc_sync_state(struct device *dev)
> >>>>>>> +{
> >>>>>>> +	icc_sync_state(dev);
> >>>>>> Only if desc->icc_hws != 0, otherwise it will mess the interconnect
> >>>>>> internals. You might need to set drvdata to desc.
> >>>>>
> >>>>> Hmm…
> >>>>>
> >>>>> Currently icc_sync_state does not seem to use the dev argument at all.
> >>>>>
> >>>>> How would something get messed up, now or whenever icc_sync_state changes?
> >>>>> o.0
> >>>>
> >>>> Yes :-(
> >>>
> >>> Sorry about the delayed response since I was out of town all last week.
> >>> Just to be clear, the missing check for 'desc->icc_hws != 0' is a bug that
> >>> existed prior to my change, and I should label it as such with a Fixes
> >>> tag when I post my next version?
> >>
> >> Up until this change, having icc_hws > 0 but lacking icc_sync_state
> >> (or the reverse) would be be considered programmer error
> > 
> > icc_hws > 0 but lacking icc_sync_state (or the reverse) makes sense as a
> > programmer error. However...
> > 
> >> Starting with patch 4, this gets assigned unconditionally, so there's
> >> no prior bug to be fixed
> > 
> > I don't see where that situation happens here. All of the places where
> > icc_sync_state() was previously called, the new code now calls
> > qcom_cc_sync_state() -> icc_sync_state(). (There is
> > qcom_msm8996_cbf_icc_sync_state() that needs to be modified.)
> > 
> > In patch 4 of this series, it sets up a framework level sync_state()
> > callback with dev_set_drv_sync_state(). If a sync_state already exists,
> > then that call will fail with -EBUSY, and it will leave the existing
> > sync_state() intact. So it's not calling sync_state twice. I will
> > clarify that on the comment.
> 
> Dmitry and I are referring to the situation where the clock driver isn't
> an interconnect provider but icc_sync_state() still executes. That could
> not have been the case before, since most clock drivers didn't come with
> any sort of .sync_state()

I'm sorry if I am being really dense here.

Let's ignore clk-cbf-8996.c since that has a separate issue. The other 6
drivers in this patch today all have this pattern:

static struct platform_driver foo_driver = {
	...
        .driver = {
		...
                .sync_state = icc_sync_state,
        },
};

I'm changing it to call qcom_cc_sync_state(), which will call
icc_sync_state(). So everywhere where icc_sync_state() is called today
it will still be called after the series is applied.

All of the other clk drivers will just call clk_sync_state() directly
that's set at the framework level.

Brian


^ permalink raw reply

* Re: [PATCH 3/4] clk: qcom: convert from icc_sync_state() to qcom_cc_sync_state()
From: Jens Glathe @ 2026-06-15 14:50 UTC (permalink / raw)
  To: Brian Masney
  Cc: Saravana Kannan, Abel Vesa, Maxime Ripard, Michael Turquette,
	Stephen Boyd, Russell King, Bjorn Andersson, Hans de Goede,
	linux-clk, linux-kernel, linux-arm-msm
In-Reply-To: <ajAKrYK44-C9B9Yp@redhat.com>

Hi Brian,

On 6/15/26 16:22, Brian Masney wrote:
> Thanks for testing. I'm going to put the Tested-by on patch 4 that actually
> adds the sync_state support when I post the new version unless I hear
> from you otherwise.

I would say the Tested-by should be on the whole patch series as its 
running here.

with best regards

Jens


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox