* Re: [PATCH 55/61] interconnect: Prefer IS_ERR_OR_NULL over manual NULL check
From: Krzysztof Kozlowski @ 2026-04-16 12:24 UTC (permalink / raw)
To: Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel,
dri-devel, gfs2, intel-gfx, intel-wired-lan, iommu, kvm,
linux-arm-kernel, linux-block, linux-bluetooth, linux-btrfs,
linux-cifs, linux-clk, linux-erofs, linux-ext4, linux-fsdevel,
linux-gpio, linux-hyperv, linux-input, linux-kernel, linux-leds,
linux-media, linux-mips, linux-mm, linux-modules, linux-mtd,
linux-nfs, linux-omap, linux-phy, linux-pm, linux-rockchip,
linux-s390, linux-scsi, linux-sctp, linux-security-module,
linux-sh, linux-sound, linux-stm32, linux-trace-kernel, linux-usb,
linux-wireless, netdev, ntfs3, samba-technical, sched-ext,
target-devel, tipc-discussion, v9fs
Cc: Georgi Djakov
In-Reply-To: <20260310-b4-is_err_or_null-v1-55-bd63b656022d@avm.de>
On 10/03/2026 12:49, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Semantich change: Previously the code only printed the warning on error,
> but not when the pointer was NULL. Now the warning is printed in both
> cases!
NAK, read the code
>
> Change found with coccinelle.
>
> To: Georgi Djakov <djakov@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> drivers/interconnect/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 8569b78a18517b33abeafac091978b25cbc1acc7..22e92b30f73853d5bd2e05b4f52cb5aa22556468 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -790,7 +790,7 @@ void icc_put(struct icc_path *path)
> size_t i;
> int ret;
>
> - if (!path || WARN_ON(IS_ERR(path)))
> + if (WARN_ON(IS_ERR_OR_NULL(path)))
IS_ERR_OR_NULL is simply discouraged, but beside of code preference, you
just added bug here. This is clearly not equivalent and you emit warn on
perfectly valid case!
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 02/13] dt-bindings: leds: document Samsung S2M series PMIC RGB LED device
From: Kaustabh Chakraborty @ 2026-04-16 12:15 UTC (permalink / raw)
To: Krzysztof Kozlowski, Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
André Draszik, Alexandre Belloni, Jonathan Corbet,
Shuah Khan, Nam Tran, Łukasz Lebiedziński, linux-leds,
devicetree, linux-kernel, linux-pm, linux-samsung-soc, linux-rtc,
linux-doc
In-Reply-To: <20260416-upbeat-archetypal-mantis-1ede48@quoll>
On 2026-04-16 10:23 +02:00, Krzysztof Kozlowski wrote:
> On Wed, Apr 15, 2026 at 11:00:16PM +0530, Kaustabh Chakraborty wrote:
>> On 2026-04-15 09:03 +02:00, Krzysztof Kozlowski wrote:
>> > On Tue, Apr 14, 2026 at 12:02:54PM +0530, Kaustabh Chakraborty wrote:
>> >> +description: |
>> >> + The Samsung S2M series PMIC RGB LED is a three-channel LED device with
>> >> + 8-bit brightness control for each channel, typically used as status
>> >> + indicators in mobile phones.
>> >> +
>> >> + This is a part of device tree bindings for S2M and S5M family of Power
>> >> + Management IC (PMIC).
>> >> +
>> >> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
>> >> + additional information and example.
>> >> +
>> >> +allOf:
>> >> + - $ref: common.yaml#
>> >
>> > Rob's comment is still valid:
>> > 1. How do you address one of three LEDs in non-RGB case?
>> > 2. Where is multi-color?
>>
>> Yes, multi-color should have been added here.
>>
>> >
>> > And based on this alone without other properties, I say this should be
>> > part of top-level schema. Separate node is fine, but no need for
>> > separate binding.
>>
>> BTW, for loading the sub-device driver via platform (as it won't be a
>> separate binding) the driver *must* be built-in. Although not related to
>> bindings, this seems counter-intuitive. I see the same problem with the
>
> I don't understand that comment. If it has nothing to do with the
> binding, what is the problem?
It was an unrelated user-space issue, so ignore.
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: power: Add power-domains-child-ids property
From: Rob Herring (Arm) @ 2026-04-16 12:03 UTC (permalink / raw)
To: Kevin Hilman (TI)
Cc: devicetree, Ulf Hansson, linux-arm-kernel, linux-pm, linux-kernel,
arm-scmi, Geert Uytterhoeven
In-Reply-To: <20260410-topic-lpm-pmdomain-child-ids-v2-1-83396e4b5f8b@baylibre.com>
On Fri, 10 Apr 2026 16:44:36 -0700, Kevin Hilman (TI) wrote:
> Add binding documentation for the new power-domains-child-ids property,
> which works in conjunction with the existing power-domains property to
> establish parent-child relationships between a multi-domain power domain
> provider and external parent domains.
>
> Each element in the uint32 array identifies the child domain
> ID (index) within the provider that should be made a child domain of
> the corresponding phandle entry in power-domains. The two arrays must
> have the same number of elements.
>
> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
> ---
> Documentation/devicetree/bindings/power/power-domain.yaml | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] dt-bindings: thermal: Fix false warning with 'phandle' in trips nodes
From: Rob Herring (Arm) @ 2026-04-16 12:03 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Daniel Lezcano, Zhang Rui, Krzysztof Kozlowski, Lukasz Luba,
Rafael J. Wysocki, linux-pm, Conor Dooley, linux-kernel,
devicetree
In-Reply-To: <20260410223601.1487473-2-robh@kernel.org>
On Fri, 10 Apr 2026 17:36:00 -0500, Rob Herring (Arm) wrote:
> A pattern property matching essentially anything doesn't work if there
> are implicit properties such as 'phandle' which can occur on any node.
> One such example popped up recently:
>
> arch/arm64/boot/dts/qcom/sm8650-hdk.dtb: thermal-zones: gpuss0-thermal:trips:phandle: 531 is not of type 'object'
> from schema $id: http://devicetree.org/schemas/thermal/thermal-zones.yaml
>
> Instead of a pattern property, use an "additionalProperties" schema
> instead which is the fallback in case of no matching property.
>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> Daniel, Please pick this up for v7.1 as the above warning is in next. Or
> if you prefer, I can take it.
>
> .../bindings/thermal/thermal-zones.yaml | 111 +++++++++---------
> 1 file changed, 54 insertions(+), 57 deletions(-)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 2/2] riscv: dts: spacemit: Add cpu scaling for K1 SoC
From: Anand Moon @ 2026-04-16 11:37 UTC (permalink / raw)
To: Shuwei Wu
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Yixun Lan, linux-pm, linux-kernel, linux-riscv,
spacemit, devicetree
In-Reply-To: <DHUCL24GMX7D.369IWK9DLPZPX@mailbox.org>
Hi Shuwei,
Thanks for sharing the details.
On Thu, 16 Apr 2026 at 11:29, Shuwei Wu <shuwei.wu@mailbox.org> wrote:
>
> On Tue Apr 14, 2026 at 9:25 PM CST, Anand Moon wrote:
> > Hi Shuwei,
> >
> > On Fri, 10 Apr 2026 at 13:30, Shuwei Wu <shuwei.wu@mailbox.org> wrote:
> >>
> >> Add Operating Performance Points (OPP) tables and CPU clock properties
> >> for the two clusters in the SpacemiT K1 SoC.
> >>
> >> Also assign the CPU power supply (cpu-supply) for the Banana Pi BPI-F3
> >> board to fully enable CPU DVFS.
> >>
> >> Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
> >>
> >> ---
> >> Changes in v2:
> >> - Add k1-opp.dtsi with OPP tables for both CPU clusters
> >> - Assign CPU supplies and include OPP table for Banana Pi BPI-F3
> >> ---
> >> arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 35 +++++++-
> >> arch/riscv/boot/dts/spacemit/k1-opp.dtsi | 105 ++++++++++++++++++++++++
> >> arch/riscv/boot/dts/spacemit/k1.dtsi | 8 ++
> >> 3 files changed, 147 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> >> index 444c3b1e6f44..3780593f610d 100644
> >> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> >> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> >> @@ -5,6 +5,7 @@
> >>
> >> #include "k1.dtsi"
> >> #include "k1-pinctrl.dtsi"
> >> +#include "k1-opp.dtsi"
> >>
> >> / {
> >> model = "Banana Pi BPI-F3";
> >> @@ -86,6 +87,38 @@ &combo_phy {
> >> status = "okay";
> >> };
> >>
> >> +&cpu_0 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_1 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_2 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_3 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_4 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_5 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_6 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> +&cpu_7 {
> >> + cpu-supply = <&buck1_3v45>;
> >> +};
> >> +
> >> &emmc {
> >> bus-width = <8>;
> >> mmc-hs400-1_8v;
> >> @@ -201,7 +234,7 @@ pmic@41 {
> >> dldoin2-supply = <&buck5>;
> >>
> >> regulators {
> >> - buck1 {
> >> + buck1_3v45: buck1 {
> >> regulator-min-microvolt = <500000>;
> >> regulator-max-microvolt = <3450000>;
> >> regulator-ramp-delay = <5000>;
> >> diff --git a/arch/riscv/boot/dts/spacemit/k1-opp.dtsi b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
> >> new file mode 100644
> >> index 000000000000..768ae390686d
> >> --- /dev/null
> >> +++ b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
> >> @@ -0,0 +1,105 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +
> >> +/ {
> >> + cluster0_opp_table: opp-table-cluster0 {
> >> + compatible = "operating-points-v2";
> >> + opp-shared;
> >> +
> >> + opp-614400000 {
> >> + opp-hz = /bits/ 64 <614400000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-819000000 {
> >> + opp-hz = /bits/ 64 <819000000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1000000000 {
> >> + opp-hz = /bits/ 64 <1000000000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1228800000 {
> >> + opp-hz = /bits/ 64 <1228800000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1600000000 {
> >> + opp-hz = /bits/ 64 <1600000000>;
> >> + opp-microvolt = <1050000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> + };
> >> +
> >> + cluster1_opp_table: opp-table-cluster1 {
> >> + compatible = "operating-points-v2";
> >> + opp-shared;
> >> +
> >> + opp-614400000 {
> >> + opp-hz = /bits/ 64 <614400000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-819000000 {
> >> + opp-hz = /bits/ 64 <819000000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1000000000 {
> >> + opp-hz = /bits/ 64 <1000000000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1228800000 {
> >> + opp-hz = /bits/ 64 <1228800000>;
> >> + opp-microvolt = <950000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> +
> >> + opp-1600000000 {
> >> + opp-hz = /bits/ 64 <1600000000>;
> >> + opp-microvolt = <1050000>;
> >> + clock-latency-ns = <200000>;
> >> + };
> >> + };
> >> +};
> >> +
> >> +&cpu_0 {
> >> + operating-points-v2 = <&cluster0_opp_table>;
> >> +};
> >> +
> >> +&cpu_1 {
> >> + operating-points-v2 = <&cluster0_opp_table>;
> >> +};
> >> +
> >> +&cpu_2 {
> >> + operating-points-v2 = <&cluster0_opp_table>;
> >> +};
> >> +
> >> +&cpu_3 {
> >> + operating-points-v2 = <&cluster0_opp_table>;
> >> +};
> >> +
> >> +&cpu_4 {
> >> + operating-points-v2 = <&cluster1_opp_table>;
> >> +};
> >> +
> >> +&cpu_5 {
> >> + operating-points-v2 = <&cluster1_opp_table>;
> >> +};
> >> +
> >> +&cpu_6 {
> >> + operating-points-v2 = <&cluster1_opp_table>;
> >> +};
> >> +
> >> +&cpu_7 {
> >> + operating-points-v2 = <&cluster1_opp_table>;
> >> +};
> >> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> >> index 529ec68e9c23..bdd109b81730 100644
> >> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> >> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> >> @@ -54,6 +54,7 @@ cpu_0: cpu@0 {
> >> compatible = "spacemit,x60", "riscv";
> >> device_type = "cpu";
> >> reg = <0>;
> >> + clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
> >> riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
> >> riscv,isa-base = "rv64i";
> >> riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
> >> @@ -84,6 +85,7 @@ cpu_1: cpu@1 {
> >> compatible = "spacemit,x60", "riscv";
> >> device_type = "cpu";
> >> reg = <1>;
> >> + clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
> >
> > Based on the Spacemit kernel source, the k1-x_opp_table.dtsi file
> > defines several additional clocks for the Operating Performance Points
> > (OPP) table:
> >
> > clocks = <&ccu CLK_CPU_C0_ACE>, <&ccu CLK_CPU_C1_ACE>, <&ccu CLK_CPU_C0_TCM>,
> > <&ccu CLK_CCI550>, <&ccu CLK_PLL3>, <&ccu
> > CLK_CPU_C0_HI>, <&ccu CLK_CPU_C1_HI>;
> > clock-names = "ace0","ace1","tcm","cci","pll3", "c0hi", "c1hi";
> >
> > These hardware clocks are also explicitly registered in the APMU clock driver
> > via the k1_ccu_apmu_hws array, confirming their availability for frequency
> > and voltage scaling on the K1-X SoC.
> >
> > static struct clk_hw *k1_ccu_apmu_hws[] = {
> > [CLK_CCI550] = &cci550_clk.common.hw,
> > [CLK_CPU_C0_HI] = &cpu_c0_hi_clk.common.hw,
> > [CLK_CPU_C0_CORE] = &cpu_c0_core_clk.common.hw,
> > [CLK_CPU_C0_ACE] = &cpu_c0_ace_clk.common.hw,
> > [CLK_CPU_C0_TCM] = &cpu_c0_tcm_clk.common.hw,
> > [CLK_CPU_C1_HI] = &cpu_c1_hi_clk.common.hw,
> > [CLK_CPU_C1_CORE] = &cpu_c1_core_clk.common.hw,
> > [CLK_CPU_C1_ACE] = &cpu_c1_ace_clk.common.hw,
> >
> > Yes, it is possible to add these clocks for DVFS to work correctly,
> > provided they are managed by the appropriate driver and declared in
> > the Device Tree (DT).
> >
> > Thanks
> > -Anand
>
> Thanks for your review and for pointing this out.
>
> Regarding the clocks you mentioned, I'd like to clarify their roles based on
> the K1 datasheet. Taking Cluster 0 as an example, c0_core_clk is the primary
> clock for the cluster. c0_ace_clk and c0_tcm_clk are children derived from it,
> defaulting to half the frequency of their parent core clock, while c0_hi_clk
> represents the high-speed path selection.
> Cluster 1 follows the same structure.
>
> Based on the official SpacemiT Bianbu OS source, the spacemit-cpufreq.c driver
> mainly performs the following tasks:
> 1. Sets the CCI550 clock frequency to 614MHz.
> 2. Sets the clock frequencies of c0_ace_clk, c1_ace1_clk, and c0_tcm_clk to half
> the frequency of their parent clock.
> 3. For the 1.6GHz OPP, it sets the PLL3 frequency to 3.2GHz and the
> c0_hi_clk/c1_hi_clk frequencies to 1.6GHz.
>
> I booted with the manufacturer's OpenWRT image and used debugfs to confirm that
> the clock states are exactly as described above.
>
> At 1.6GHz:
> Clock Source & Tree Rate (Hz) HW Enable Consumer
> ---------------------------------------------------------------------------
> pll3 3,200,000,000 Y deviceless
> └─ pll3_d2 1,600,000,000 Y deviceless
> ├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
> │ └─ cpu_c1_pclk 1,600,000,000 Y cpu0
> │ └─ cpu_c1_ace_clk 800,000,000 Y deviceless
> └─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
> └─ cpu_c0_core_clk 1,600,000,000 Y cpu0
> ├─ cpu_c0_tcm_clk 800,000,000 Y deviceless
> └─ cpu_c0_ace_clk 800,000,000 Y deviceless
>
> pll1_2457p6_vco 2,457,600,000 Y deviceless
> └─ pll1_d4 614,400,000 Y deviceless
> └─ pll1_d4_614p4 614,400,000 Y deviceless
> └─ cci550_clk 614,400,000 Y deviceless
>
> At 1.228GHz:
> Clock Source & Tree Rate (Hz) HW Enable Consumer
> ---------------------------------------------------------------------------
> pll1_2457p6_vco 2,457,600,000 Y deviceless
> └─ pll1_d2 1,228,800,000 Y deviceless
> └─ pll1_d2_1228p8 1,228,800,000 Y deviceless
> ├─ cpu_c0_core_clk 1,228,800,000 Y cpu0
> │ ├─ cpu_c0_tcm_clk 614,400,000 Y deviceless
> │ └─ cpu_c0_ace_clk 614,400,000 Y deviceless
> └─ cpu_c1_pclk 1,228,800,000 Y cpu0
> └─ cpu_c1_ace_clk 614,400,000 Y deviceless
> └─ pll1_d4 614,400,000 Y deviceless
> └─ pll1_d4_614p4 614,400,000 Y deviceless
> └─ cci550_clk 614,400,000 Y deviceless
>
> pll3 3,200,000,000 Y deviceless
> └─ pll3_d2 1,600,000,000 Y deviceless
> ├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
> └─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
> └─ pll3_d3 1,066,666,666 Y deviceless
>
> Regarding the necessity of listing these clocks in the DT, my analysis is as follows:
> 1. For CCI550, I did not find a clear definition of this clock's specific role
> in the SoC datasheet. Although the vendor kernel increases its frequency,
> my benchmarks show that maintaining the mainline default (245.76MHz) has a
> negligible impact on CPU performance.
> 2. For ACE and TCM clocks, they function as synchronous children of the core
> clock with a default divide-by-2 ratio. Since they scale automatically relative
> to c0_core_clk/c1_core_clk and no other peripherals depend on them, they do not
> require manual management in the OPP table.
> 3. For the high-speed path, the underlying clock controller logic already handles
> the parent MUX switching and PLL3 scaling automatically when clk_set_rate()
> is called on the core clock.
>
> I have verified this by checking the hardware state in the mainline kernel.
> The clock tree matches the vendor kernel's configuration:
>
> At 1.6GHz:
> Clock Source & Tree Rate (Hz) HW Enable Consumer
> ---------------------------------------------------------------------------
> pll3 3,200,000,000 Y deviceless
> └─ pll3_d2 1,600,000,000 Y deviceless
> ├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
> │ └─ cpu_c1_core_clk 1,600,000,000 Y cpu4
> │ └─ cpu_c1_ace_clk 800,000,000 Y deviceless
> └─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
> └─ cpu_c0_core_clk 1,600,000,000 Y cpu0
> ├─ cpu_c0_tcm_clk 800,000,000 Y deviceless
> └─ cpu_c0_ace_clk 800,000,000 Y deviceless
>
> pll1 2,457,600,000 Y deviceless
> └─ pll1_d5 491,520,000 Y deviceless
> └─ pll1_d5_491p52 491,520,000 Y deviceless
> └─ cci550_clk 245,760,000 Y deviceless
>
> At 1.228GHz:
> Clock Source & Tree Rate (Hz) HW Enable Consumer
> ---------------------------------------------------------------------------
> pll1 2,457,600,000 Y deviceless
> ├─ pll1_d5 491,520,000 Y deviceless
> │ └─ pll1_d5_491p52 491,520,000 Y deviceless
> │ └─ cci550_clk 245,760,000 Y deviceless
> └─ pll1_d2 1,228,800,000 Y deviceless
> └─ pll1_d2_1228p8 1,228,800,000 Y deviceless
> └─ cpu_c0_core_clk 1,228,800,000 Y cpu0
> ├─ cpu_c0_tcm_clk 614,400,000 Y deviceless
> └─ cpu_c0_ace_clk 614,400,000 Y deviceless
>
> pll3 3,200,000,000 Y deviceless
> └─ pll3_d2 1,600,000,000 Y deviceless
> └─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
> └─ cpu_c1_core_clk 1,600,000,000 Y cpu4
> └─ cpu_c1_ace_clk 800,000,000 Y deviceless
>
Thanks, I have verified the clocks are set to Y in
/sys/kernel/debug/clk/clk_summary
> Performance benchmarks also confirm that the current configuration is sufficient:
> Benchmark (AWK computation): time awk 'BEGIN{for(i=0;i<10000000;i++) sum+=i}'
> ----------------------------------------------------------------------------
> Frequency | Mainline Linux (s) | OpenWrt (s)
> (kHz) | Real (Total) | User (CPU) | Real (Total) | User (CPU) )
> -------------+---------------+---------------+---------------+--------------
> 1,600,000 | 1.82s | 1.81s | 1.73s | 1.73s
> 1,228,800 | 2.34s | 2.33s | 2.26s | 2.26s
> 1,000,000 | 2.94s | 2.86s | 2.78s | 2.78s
> 819,000 | 3.54s | 3.53s | 3.39s | 3.39s
> 614,400 | 4.73s | 4.71s | 4.51s | 4.51s
> ----------------------------------------------------------------------------
>
> In summary, because the clock controller correctly handles the internal dividers
> and parent switching, declaring only the primary core clock for each CPU node is
> sufficient for functional DVFS.
>
I have just tested this patch against next-20260415
But, I have observed this log on the Banana Pi F3 dev board with the
Banana PI - R4 heat sink and fan.
[ 5.803445][ T1] In-situ OAM (IOAM) with IPv6
[ 5.809605][ T1] NET: Registered PF_PACKET protocol family
[ 5.819098][ T1] Key type dns_resolver registered
[ 5.853430][ C2] cpu2: scalar unaligned word access speed is
1.60x byte access speed (fast)
[ 5.853431][ C3] cpu3: scalar unaligned word access speed is
1.67x byte access speed (fast)
[ 5.853440][ C7] cpu7: scalar unaligned word access speed is
8.10x byte access speed (fast)
[ 5.853432][ C1] cpu1: scalar unaligned word access speed is
3.98x byte access speed (fast)
[ 5.853431][ T1] cpu0: scalar unaligned word access speed is
2.33x byte access speed (fast)
[ 5.853436][ C5] cpu5: scalar unaligned word access speed is
2.29x byte access speed (fast)
[ 5.853436][ C6] cpu6: scalar unaligned word access speed is
2.58x byte access speed (fast)
[ 5.853431][ C4] cpu4: scalar unaligned word access speed is
2.07x byte access speed (fast)
[ 5.936544][ T92] mmcblk0boot0: mmc0:0001 AJTD4R 4.00 MiB
[ 6.003120][ T92] mmcblk0boot1: mmc0:0001 AJTD4R 4.00 MiB
[ 6.070909][ T92] mmcblk0rpmb: mmc0:0001 AJTD4R 4.00 MiB, chardev (244:0)
[ 6.380324][ T1] registered taskstats version 1
[ 6.407337][ T1] Loading compiled-in X.509 certificates
[ 6.594206][ T1] Loaded X.509 cert 'Build time autogenerated
kernel key: 19b81ec48e45e6ee983623417bad5096df8bbcf1'
[ 7.600343][ T1] Demotion targets for Node 0: null
[ 7.608583][ T1] kmemleak: Kernel memory leak detector
initialized (mem pool available: 1309)
[ 7.608646][ T120] kmemleak: Automatic memory scanning thread started
[ 7.624663][ T1] debug_vm_pgtable: [debug_vm_pgtable ]:
Validating architecture page table helpers
[ 7.636721][ T1] page_owner is disabled
[ 8.213648][ T74] debugfs: ':soc:gpio@d4019000-1' already exists
in 'domains'
[ 8.233502][ T74] debugfs: ':soc:gpio@d4019000-1' already exists
in 'domains'
[ 8.254012][ T74] debugfs: ':soc:gpio@d4019000-1' already exists
in 'domains'
[ 8.319431][ T74] printk: legacy console [ttyS0] disabled
[ 8.345811][ T74] d4017000.serial: ttyS0 at MMIO 0xd4017000 (irq
= 16, base_baud = 921600) is a XScale
[ 8.357331][ T74] printk: legacy console [ttyS0] enabled
[ 8.357331][ T74] printk: legacy console [ttyS0] enabled
[ 8.369971][ T74] printk: legacy bootconsole [uart0] disabled
[ 8.369971][ T74] printk: legacy bootconsole [uart0] disabled
[ 8.427040][ T74] /soc/i2c@d401d800/pmic@41: Fixed dependency
cycle(s) with /soc/i2c@d401d800/pmic@41/regulators/buck5
[ 8.634595][ T74] spacemit-p1-rtc spacemit-p1-rtc.1.auto:
registered as rtc0
[ 8.642732][ T74] spacemit-p1-rtc spacemit-p1-rtc.1.auto: setting
system clock to 2026-04-10T00:03:42 UTC (1775779422)
[ 8.766081][ T74] sdhci-spacemit d4280000.mmc: Got CD GPIO
[ 8.801855][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 8.806411][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 8.813413][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 8.818261][ T130] cpu cpu4: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 8.818307][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 8.827239][ T129] cpu cpu0: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 8.833161][ T130] cpu cpu4: Failed to set regulator voltages: -22
[ 8.842546][ T129] cpu cpu0: Failed to set regulator voltages: -22
[ 8.848941][ T130] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 8.855273][ T129] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 8.893720][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 8.904437][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 8.908515][ T129] cpu cpu0: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 8.918057][ T129] cpu cpu0: Failed to set regulator voltages: -22
[ 8.924402][ T129] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 8.945668][ T74] mmc1: SDHCI controller on d4280000.mmc
[d4280000.mmc] using ADMA
[ 8.976207][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 8.980156][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 8.986169][ T130] cpu cpu4: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 8.995473][ T130] cpu cpu4: Failed to set regulator voltages: -22
[ 9.001603][ T130] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.020028][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.024051][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.030122][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.036004][ T130] cpu cpu4: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.036059][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.045003][ T130] cpu cpu4: Failed to set regulator voltages: -22
[ 9.045077][ T130] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.058079][ T57] spacemit-k1-pcie ca800000.pcie: host bridge
/soc/pcie-bus/pcie@ca800000 ranges:
[ 9.064716][ T129] cpu cpu0: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.064745][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.064825][ T129] cpu cpu0: Failed to set regulator voltages: -22
[ 9.064889][ T129] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.065762][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.069924][ T60] spacemit-k1-pcie ca400000.pcie: host bridge
/soc/pcie-bus/pcie@ca400000 ranges:
[ 9.071122][ T60] spacemit-k1-pcie ca400000.pcie: IO
0x009f002000..0x009f101fff -> 0x0000000000
[ 9.073304][ T60] spacemit-k1-pcie ca400000.pcie: MEM
0x0090000000..0x009effffff -> 0x0090000000
[ 9.081407][ T74] at24 2-0050: 256 byte 24c02 EEPROM, read-only
[ 9.083047][ T130] cpu cpu4: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.083509][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.083614][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.083686][ T129] cpu cpu0: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.083845][ T129] cpu cpu0: Failed to set regulator voltages: -22
[ 9.083885][ T129] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.089945][ T57] spacemit-k1-pcie ca800000.pcie: IO
0x00b7002000..0x00b7101fff -> 0x0000000000
[ 9.092728][ T1] clk: Disabling unused clocks
[ 9.095269][ T130] cpu cpu4: Failed to set regulator voltages: -22
[ 9.096981][ T1] PM: genpd: Disabling unused power domains
[ 9.104949][ T57] spacemit-k1-pcie ca800000.pcie: MEM
0x00a0000000..0x00afffffff -> 0x00a0000000
[ 9.107986][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.108166][ T129] buck1: Restricting voltage, 1050000-950000uV
[ 9.108246][ T129] cpu cpu0: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.108311][ T129] cpu cpu0: Failed to set regulator voltages: -22
[ 9.108356][ T129] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.108582][ T130] cpufreq: __target_index: Failed to change cpu
frequency: -22
[ 9.113366][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.118144][ T57] spacemit-k1-pcie ca800000.pcie: MEM
0x00b0000000..0x00b6ffffff -> 0x00b0000000
[ 9.130386][ T130] buck1: Restricting voltage, 1050000-950000uV
[ 9.196246][ T130] cpu cpu4: _set_opp_voltage: failed to set
voltage (1050000 1050000 1050000 mV): -22
[ 9.202562][ T60] spacemit-k1-pcie ca400000.pcie: iATU: unroll T,
8 ob, 8 ib, align 4K, limit 4G
[ 9.206998][ T130] cpu cpu4: Failed to set regulator voltages: -22
[ 9.257180][ T130] cpufreq: __target_index: Failed to change cpu
frequency: -22
After reviewing the Banana Pi F3 schematics, I confirmed that Buck1 and Buck2
Both supply the CORE_0V9 with 0.9V±1% rail. To resolve the restriction errors,
I expanded the voltage range in the DTS to 500,000–950,000 µV.
Additionally, I updated the DTS to map the second CPU cluster (cores 4–7)
to Buck2 to better align with the hardware's power distribution.
[1] https://drive.google.com/file/d/19iLJ5xnCB_oK8VeQjkPGjzAn39WYyylv/view
(page 4)
Can you share your thoughts on the changes below?
$ git diff
diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
index 7e300cca50d8..be53645ba0c6 100644
--- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
@@ -102,19 +102,19 @@ &cpu_3 {
};
&cpu_4 {
- cpu-supply = <&buck1_3v45>;
+ cpu-supply = <&buck2_3v45>;
};
&cpu_5 {
- cpu-supply = <&buck1_3v45>;
+ cpu-supply = <&buck2_3v45>;
};
&cpu_6 {
- cpu-supply = <&buck1_3v45>;
+ cpu-supply = <&buck2_3v45>;
};
&cpu_7 {
- cpu-supply = <&buck1_3v45>;
+ cpu-supply = <&buck2_3v45>;
};
&emmc {
@@ -234,14 +234,14 @@ pmic@41 {
regulators {
buck1_3v45: buck1 {
regulator-min-microvolt = <500000>;
- regulator-max-microvolt = <3450000>;
+ regulator-max-microvolt = <950000>;
regulator-ramp-delay = <5000>;
regulator-always-on;
};
- buck2 {
+ buck2_3v45: buck2 {
regulator-min-microvolt = <500000>;
- regulator-max-microvolt = <3450000>;
+ regulator-max-microvolt = <950000>;
regulator-ramp-delay = <5000>;
regulator-always-on;
};
> --
> Best regards,
> Shuwei Wu
Thanks
-Anand
^ permalink raw reply related
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Petr Pavlu @ 2026-04-16 11:18 UTC (permalink / raw)
To: Song Chen
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe, jikos, mbenes,
pmladek, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <a35f5f94-7d5a-4347-974b-b270c89ef241@189.cn>
On 4/15/26 8:43 AM, Song Chen wrote:
> On 4/14/26 22:33, Petr Pavlu wrote:
>> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>> index 14f391b186c6..0bdd56f9defd 100644
>>> --- a/include/linux/module.h
>>> +++ b/include/linux/module.h
>>> @@ -308,6 +308,14 @@ enum module_state {
>>> MODULE_STATE_COMING, /* Full formed, running module_init. */
>>> MODULE_STATE_GOING, /* Going away. */
>>> MODULE_STATE_UNFORMED, /* Still setting it up. */
>>> + MODULE_STATE_FORMED,
>>
>> I don't see a reason to add a new module state. Why is it necessary and
>> how does it fit with the existing states?
>>
> because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace has someting to do in this state), notifier chain will roll back by calling blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going to jeopardise the notifers which don't handle it appropriately, like:
>
> case MODULE_STATE_COMING:
> kmalloc();
> case MODULE_STATE_GOING:
> kfree();
My understanding is that the current module "state machine" operates as
follows. Transitions marked with an asterisk (*) are announced via the
module notifier.
---> UNFORMED --*> COMING --*> LIVE --*> GOING -.
^ | ^ |
| '---------------------* |
'---------------------------------------'
The new code aims to replace the current ftrace_module_init() call in
load_module(). To achieve this, it adds a notification for the UNFORMED
state (only when loading a module) and introduces a new FORMED state for
rollback. FORMED is purely a fake state because it never appears in
module::state. The new structure is as follows:
,--*> (FORMED)
|
--*> UNFORMED --*> COMING --*> LIVE --*> GOING -.
^ | ^ |
| '---------------------* |
'---------------------------------------'
I'm afraid this is quite complex and inconsistent. Unless it can be kept
simple, we would be just replacing one special handling with a different
complexity, which is not worth it.
>>
>>> + if (err)
>>> + goto ddebug_cleanup;
>>> /* Finally it's fully formed, ready to start executing. */
>>> err = complete_formation(mod, info);
>>> - if (err)
>>> + if (err) {
>>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>>> + MODULE_STATE_FORMED, mod);
>>> goto ddebug_cleanup;
>>> + }
>>> - err = prepare_coming_module(mod);
>>> + err = prepare_module_state_transaction(mod,
>>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
>>> if (err)
>>> goto bug_cleanup;
>>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>> destroy_params(mod->kp, mod->num_kp);
>>> blocking_notifier_call_chain(&module_notify_list,
>>> MODULE_STATE_GOING, mod);
>>
>> My understanding is that all notifier chains for MODULE_STATE_GOING
>> should be reversed.
> yes, all, from lowest priority notifier to highest.
> I will resend patch 1 which was failed due to my proxy setting.
What I meant here is that the call:
blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_GOING, mod);
should be replaced with:
blocking_notifier_call_chain_reverse(&module_notify_list, MODULE_STATE_GOING, mod);
>
>>
>>> - klp_module_going(mod);
>>> bug_cleanup:
>>> mod->state = MODULE_STATE_GOING;
>>> /* module_bug_cleanup needs module_mutex protection */
>>
>> The patch removes the klp_module_going() cleanup call in load_module().
>> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
>> should be removed and appropriately replaced with a cleanup via
>> a notifier.
>>
> err = prepare_module_state_transaction(mod,
> MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
> if (err)
> goto ddebug_cleanup;
>
> ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
>
> err = prepare_module_state_transaction(mod,
> MODULE_STATE_COMING, MODULE_STATE_GOING);
>
> each notifier including ftrace and klp will be cleanup in blocking_notifier_call_chain_robust rolling back.
>
> if all notifiers are successful in MODULE_STATE_COMING, they all will be clean up in
> coming_cleanup:
> mod->state = MODULE_STATE_GOING;
> destroy_params(mod->kp, mod->num_kp);
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_GOING, mod);
>
> if something wrong underneath.
My point is that the patch leaves a call to ftrace_release_mod() in
load_module(), which I expected to be handled via a notifier.
--
Thanks,
Petr
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Petr Mladek @ 2026-04-16 10:33 UTC (permalink / raw)
To: chensong_2000
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mhiramat, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260415070137.17860-1-chensong_2000@189.cn>
On Wed 2026-04-15 15:01:37, chensong_2000@189.cn wrote:
> From: Song Chen <chensong_2000@189.cn>
>
> The current notifier chain implementation uses a single-linked list
> (struct notifier_block *next), which only supports forward traversal
> in priority order. This makes it difficult to handle cleanup/teardown
> scenarios that require notifiers to be called in reverse priority order.
>
> A concrete example is the ordering dependency between ftrace and
> livepatch during module load/unload. see the detail here [1].
>
> This patch replaces the single-linked list in struct notifier_block
> with a struct list_head, converting the notifier chain into a
> doubly-linked list sorted in descending priority order. Based on
> this, a new function notifier_call_chain_reverse() is introduced,
> which traverses the chain in reverse (ascending priority order).
> The corresponding blocking_notifier_call_chain_reverse() is also
> added as the locking wrapper for blocking notifier chains.
>
> The internal notifier_call_chain_robust() is updated to use
> notifier_call_chain_reverse() for rollback: on error, it records
> the failing notifier (last_nb) and the count of successfully called
> notifiers (nr), then rolls back exactly those nr-1 notifiers in
> reverse order starting from last_nb's predecessor, without needing
> to know the total length of the chain.
>
> With this change, subsystems with symmetric setup/teardown ordering
> requirements can register a single notifier_block with one priority
> value, and rely on blocking_notifier_call_chain() for forward
> traversal and blocking_notifier_call_chain_reverse() for reverse
> traversal, without needing hard-coded call sequences or separate
> notifier registrations for each direction.
>
> [1]:https://lore.kernel.org/all
> /alpine.LNX.2.00.1602172216491.22700@cbobk.fhfr.pm/
>
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -53,41 +53,41 @@ typedef int (*notifier_fn_t)(struct notifier_block *nb,
[...]
> struct notifier_block {
> notifier_fn_t notifier_call;
> - struct notifier_block __rcu *next;
> + struct list_head __rcu entry;
> int priority;
> };
[...]
> #define ATOMIC_INIT_NOTIFIER_HEAD(name) do { \
> spin_lock_init(&(name)->lock); \
> - (name)->head = NULL; \
> + INIT_LIST_HEAD(&(name)->head); \
I would expect the RCU variant here, aka INIT_LIST_HEAD_RCU().
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -14,39 +14,47 @@
> * are layered on top of these, with appropriate locking added.
> */
>
> -static int notifier_chain_register(struct notifier_block **nl,
> +static int notifier_chain_register(struct list_head *nl,
> struct notifier_block *n,
> bool unique_priority)
> {
> - while ((*nl) != NULL) {
> - if (unlikely((*nl) == n)) {
> + struct notifier_block *cur;
> +
> + list_for_each_entry(cur, nl, entry) {
> + if (unlikely(cur == n)) {
> WARN(1, "notifier callback %ps already registered",
> n->notifier_call);
> return -EEXIST;
> }
> - if (n->priority > (*nl)->priority)
> - break;
> - if (n->priority == (*nl)->priority && unique_priority)
> +
> + if (n->priority == cur->priority && unique_priority)
> return -EBUSY;
> - nl = &((*nl)->next);
> +
> + if (n->priority > cur->priority) {
> + list_add_tail(&n->entry, &cur->entry);
> + goto out;
> + }
> }
> - n->next = *nl;
> - rcu_assign_pointer(*nl, n);
> +
> + list_add_tail(&n->entry, nl);
I would expect list_add_tail_rcu() here.
> @@ -59,25 +67,25 @@ static int notifier_chain_unregister(struct notifier_block **nl,
> * value of this parameter is -1.
> * @nr_calls: Records the number of notifications sent. Don't care
> * value of this field is NULL.
> + * @last_nb: Records the last called notifier block for rolling back
> * Return: notifier_call_chain returns the value returned by the
> * last notifier function called.
> */
> -static int notifier_call_chain(struct notifier_block **nl,
> +static int notifier_call_chain(struct list_head *nl,
> unsigned long val, void *v,
> - int nr_to_call, int *nr_calls)
> + int nr_to_call, int *nr_calls,
> + struct notifier_block **last_nb)
> {
> int ret = NOTIFY_DONE;
> - struct notifier_block *nb, *next_nb;
> -
> - nb = rcu_dereference_raw(*nl);
> + struct notifier_block *nb;
>
> - while (nb && nr_to_call) {
> - next_nb = rcu_dereference_raw(nb->next);
> + if (!nr_to_call)
> + return ret;
>
> + list_for_each_entry(nb, nl, entry) {
I would expect the RCU variant here, aka list_for_each_rcu()
These are just two random examples which I found by a quick look.
I guess that the notifier API is very old and it does not use all
the RCU API features which allow to track safety when
CONFIG_PROVE_RCU and CONFIG_PROVE_RCU_LIST are enabled.
It actually might be worth to audit the code and make it right.
> #ifdef CONFIG_DEBUG_NOTIFIERS
> if (unlikely(!func_ptr_is_kernel_text(nb->notifier_call))) {
> WARN(1, "Invalid notifier called!");
> - nb = next_nb;
> continue;
> }
> #endif
That said, I am not sure if the ftrace/livepatching handlers are
the right motivation for this. Especially when I see the
complexity of the 2nd patch [*]
After thinking more about it. I am not even sure that the ftrace and
livepatching callbacks are good candidates for generic notifiers.
They are too special. It is not only about ordering them against
each other. But it is also about ordering them against other
notifiers. The ftrace/livepatching callbacks must be the first/last
during module load/release.
[*] The 2nd patch is not archived by lore for some reason.
I have found only a review but it gives a good picture, see
https://lore.kernel.org/all/1191caf5-6a61-4622-a15e-854d3701f4fc@suse.com/
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 4/5] clk: qcom: camcc-milos: Declare icc path dependency for CAMSS_TOP_GDSC
From: Konrad Dybcio @ 2026-04-16 10:22 UTC (permalink / raw)
To: Luca Weiss, Georgi Djakov, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio
Cc: ~postmarketos/upstreaming, phone-devel, linux-pm, linux-kernel,
linux-arm-msm, linux-clk, devicetree
In-Reply-To: <20260116-milos-camcc-icc-v1-4-400b7fcd156a@fairphone.com>
On 1/16/26 2:17 PM, Luca Weiss wrote:
> This GDSC requires an interconnect path to be enabled, otherwise the
> GDSC will be stuck on 'off' and can't be enabled.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 3/8] wifi: ath10k: snoc: support powering on the device via pwrseq
From: Luca Weiss @ 2026-04-16 10:06 UTC (permalink / raw)
To: Dmitry Baryshkov, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski,
Marcel Holtmann, Luiz Augusto von Dentz, Jeff Johnson,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam, Vinod Koul,
Balakrishna Godavarthi, Matthias Kaehlcke
Cc: linux-arm-msm, linux-kernel, devicetree, linux-bluetooth,
linux-wireless, ath10k, linux-pm, Krzysztof Kozlowski,
Bartosz Golaszewski
In-Reply-To: <20260119-wcn3990-pwrctl-v3-3-948df19f5ec2@oss.qualcomm.com>
Hi Dmitry,
On Mon Jan 19, 2026 at 6:07 PM CET, Dmitry Baryshkov wrote:
> The WCN39xx family of WiFi/BT chips incorporates a simple PMU, spreading
> voltages over internal rails. Implement support for using powersequencer
> for this family of ATH10k devices in addition to using regulators.
>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/net/wireless/ath/ath10k/snoc.c | 53 ++++++++++++++++++++++++++++++++--
> drivers/net/wireless/ath/ath10k/snoc.h | 3 ++
> 2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
> index b3f6424c17d3..f72f236fb9eb 100644
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: ISC
> /*
> * Copyright (c) 2018 The Linux Foundation. All rights reserved.
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> */
>
> #include <linux/bits.h>
> @@ -11,6 +12,7 @@
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/property.h>
> +#include <linux/pwrseq/consumer.h>
> #include <linux/regulator/consumer.h>
> #include <linux/remoteproc/qcom_rproc.h>
> #include <linux/of_reserved_mem.h>
> @@ -1023,10 +1025,14 @@ static int ath10k_hw_power_on(struct ath10k *ar)
>
> ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
>
> - ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
> + ret = pwrseq_power_on(ar_snoc->pwrseq);
> if (ret)
> return ret;
>
> + ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
> + if (ret)
> + goto pwrseq_off;
> +
> ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
> if (ret)
> goto vreg_off;
> @@ -1035,18 +1041,28 @@ static int ath10k_hw_power_on(struct ath10k *ar)
>
> vreg_off:
> regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> +pwrseq_off:
> + pwrseq_power_off(ar_snoc->pwrseq);
> +
> return ret;
> }
>
> static int ath10k_hw_power_off(struct ath10k *ar)
> {
> struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
> + int ret_seq = 0;
> + int ret_vreg;
>
> ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
>
> clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
>
> - return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> + ret_vreg = regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> +
> + if (ar_snoc->pwrseq)
> + ret_seq = pwrseq_power_off(ar_snoc->pwrseq);
> +
> + return ret_vreg ? : ret_seq;
> }
>
> static void ath10k_snoc_wlan_disable(struct ath10k *ar)
> @@ -1762,7 +1778,38 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
> goto err_release_resource;
> }
>
> - ar_snoc->num_vregs = ARRAY_SIZE(ath10k_regulators);
> + /*
> + * devm_pwrseq_get() can return -EPROBE_DEFER in two cases:
> + * - it is not supposed to be used
> + * - it is supposed to be used, but the driver hasn't probed yet.
> + *
> + * There is no simple way to distinguish between these two cases, but:
> + * - if it is not supposed to be used, then regulator_bulk_get() will
> + * return all regulators as expected, continuing the probe
> + * - if it is supposed to be used, but wasn't probed yet, we will get
> + * -EPROBE_DEFER from regulator_bulk_get() too.
> + *
> + * For backwards compatibility with DTs specifying regulators directly
> + * rather than using the PMU device, ignore the defer error from
> + * pwrseq.
> + */
> + ar_snoc->pwrseq = devm_pwrseq_get(&pdev->dev, "wlan");
> + if (IS_ERR(ar_snoc->pwrseq)) {
> + ret = PTR_ERR(ar_snoc->pwrseq);
> + ar_snoc->pwrseq = NULL;
> + if (ret != -EPROBE_DEFER)
> + goto err_free_irq;
I'm fairly sure this is now broken with CONFIG_POWER_SEQUENCING=n since
then pwrseq_get() is returning ERR_PTR(-ENOSYS) which is not handled
here.
I'm observing my ath10k_snoc is now failing to probe "with error -38"
which definitely seems to be related, but I haven't debugged it further
yet.
Regards
Luca
^ permalink raw reply
* Re: [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Ulf Hansson @ 2026-04-16 9:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm,
Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
Jonathan Hunter, Dmitry Baryshkov, linux-arm-kernel, linux-kernel
In-Reply-To: <CAMuHMdWHnANr4R+AW5-xHrm=D4SJLuKVF5mq3PFkbevcTz5qWw@mail.gmail.com>
On Thu, 16 Apr 2026 at 11:15, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ulf,
>
> On Fri, 10 Apr 2026 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > Since the introduction [1] of the common sync_state support for pmdomains
> > (genpd), we have encountered a lot of various interesting problems. In most
> > cases the new behaviour of genpd triggered some weird platform specific bugs.
> >
> > That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
> > remaining limitations that we have found for genpd's sync state support. In
> > particular, we discussed the problems we have for the so-called onecell power
> > domain providers, where a single provider typically provides multiple
> > independent power domains, all with their own set of consumers.
> >
> > Note that, onecell power domain providers are very common. It's being used by
> > many SoCs/platforms/technologies. To name a few:
> > SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.
> >
> > Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
> > fine grained enough, as we end up waiting for all consumers for all power
> > domains before the ->sync_callback gets called for the supplier/provider. In
> > other words, we may end up keeping unused power domains powered-on, for no good
> > reasons.
> >
> > The series intends to fix this problem. Please have a look at the commit
> > messages for more details and help review/test!
>
> Thanks for the update!
>
> At first glance, the only real change compared to v1 seems to be
> the removal of printing
>
> pr_info("%s:%s con=%s\n", __func__, dev_name(dev),
> dev_name(consumer));
>
> Right?
Correct! I forgot to include a version history, sorry.
Besides the removed print, I have just added your tags and updated the
commit message in patch9.
FYI, my plan is to queue this as soon as v7.1-rc1 is available.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Geert Uytterhoeven @ 2026-04-16 9:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm,
Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
Jonathan Hunter, Dmitry Baryshkov, linux-arm-kernel, linux-kernel
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>
Hi Ulf,
On Fri, 10 Apr 2026 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Since the introduction [1] of the common sync_state support for pmdomains
> (genpd), we have encountered a lot of various interesting problems. In most
> cases the new behaviour of genpd triggered some weird platform specific bugs.
>
> That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
> remaining limitations that we have found for genpd's sync state support. In
> particular, we discussed the problems we have for the so-called onecell power
> domain providers, where a single provider typically provides multiple
> independent power domains, all with their own set of consumers.
>
> Note that, onecell power domain providers are very common. It's being used by
> many SoCs/platforms/technologies. To name a few:
> SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.
>
> Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
> fine grained enough, as we end up waiting for all consumers for all power
> domains before the ->sync_callback gets called for the supplier/provider. In
> other words, we may end up keeping unused power domains powered-on, for no good
> reasons.
>
> The series intends to fix this problem. Please have a look at the commit
> messages for more details and help review/test!
Thanks for the update!
At first glance, the only real change compared to v1 seems to be
the removal of printing
pr_info("%s:%s con=%s\n", __func__, dev_name(dev),
dev_name(consumer));
Right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v7 0/8] Add support for handling PCIe M.2 Key E connectors in devicetree
From: Manivannan Sadhasivam @ 2026-04-16 8:55 UTC (permalink / raw)
To: Herve Codina
Cc: Chen-Yu Tsai, Andy Shevchenko, Manivannan Sadhasivam, Rob Herring,
Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Krzysztof Kozlowski, Conor Dooley, Marcel Holtmann,
Luiz Augusto von Dentz, Bartosz Golaszewski, Bartosz Golaszewski,
linux-serial, linux-kernel, linux-kbuild, platform-driver-x86,
linux-pci, devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
Stephan Gerhold, Dmitry Baryshkov, linux-acpi, Hans de Goede,
Bartosz Golaszewski, Luca Ceresoli
In-Reply-To: <20260415165651.153b573d@bootlin.com>
On Wed, Apr 15, 2026 at 04:56:51PM +0200, Herve Codina wrote:
> Hi Chen, all,
>
> ...
>
> >
> > I'm not arguing for a even more generic "M.2" connector. The "key" is
> > already described in the compatible. I'm saying we should have some way
> > of describing the individual interfaces (PCIe, SDIO, USB, UART, I2S, I2C)
> > on the connector so further nodes or properties can be attached to them,
> > either with overlays or dynamically within the kernel. Right now the
> > are only described as individual ports, but we can't actually tie a
> > device to a OF graph port.
> >
> > But maybe I'm overthinking the representation part. AFAICT for Qualcomm's
> > UART-based BT bit part, Mani just had the driver create a device node
> > under the UART (by traversing the OF graph to find the UART). If that's
> > the desired way then the connector binding should mention it. And that
> > works for me. But I think it's messier and also we're missing an
> > opportunity to make the M.2 connector a standardized attachment point
> > for overlays.
> >
> > Mani, could you also chime in a bit on what you envisioned?
> >
> > (Added Luca from Bootlin to CC, as I think there are parallels to the
> > "Hotplug of Non-discoverable Hardware" work)
> >
>
> Related to "Hotplug of Non-discoverable Hardware",
>
> I would add entries for busses in the connector without using an OF graph.
>
I don't think this is a correct representation. It is non-standard to describe
the device nodes in some other connectors. While it may work with your series in
the future, not something I would bet-on at this point.
Using OF graph to link the connector nodes look like the cleaner solution to me.
> For I2C and later SPI, this was is done.
>
> You already have an i2c-parent property but no node where an i2c device
> can be added.
>
> The last discussion related to hotplug, connectors and DT led to the RFC
> series [1].
>
> It is a huge series. The last patch give a real example of representation:
> https://lore.kernel.org/all/20260112142009.1006236-78-herve.codina@bootlin.com/
>
> In your case I would see some thing like:
>
> connector {
> compatible = "pcie-m2-e-connector";
> vpcie3v3-supply = <&vreg_wcn_3p3>;
> vpcie1v8-supply = <&vreg_l15b_1p8>;
>
> /*
> * If those GPIOs have to be used by components available in
> * the connected board, a Nexus node should be used.
> */
> w-disable1-gpios = <&tlmm 115 GPIO_ACTIVE_LOW>;
> w-disable2-gpios = <&tlmm 116 GPIO_ACTIVE_LOW>;
> viocfg-gpios = <&tlmm 117 GPIO_ACTIVE_HIGH>;
> uart-wake-gpios = <&tlmm 118 GPIO_ACTIVE_LOW>;
> sdio-wake-gpios = <&tlmm 119 GPIO_ACTIVE_LOW>;
> sdio-reset-gpios = <&tlmm 120 GPIO_ACTIVE_LOW>;
>
> conn-i2c {
> i2c-parent = <&i2c0>;
>
> /*
> * Here i2c devices available on the board
> * connected to the connector can be described.
> */
> };
>
> /* Same kind to description for other busses */
> conn-pcie {
> pci-parent = <&xxxxx>;
>
> /*
> * The PCIe bus has abilities to discover devices.
> * Not sure this node is needed.
> *
> * If a PCI device need a DT description to describe
> * stuffs behind the device, what has been done for LAN966x
> * could be re-used [2] and [3]
> */
I don't think anyone would connect something like LAN966x to the M.2 connector.
M.2 cards have a defined purpose, like NVMe, WLAN etc... If anyone wants to
connect another SoC like LAN966x, they would use non-M.2 connectors.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v7 0/8] Add support for handling PCIe M.2 Key E connectors in devicetree
From: Manivannan Sadhasivam @ 2026-04-16 8:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Andy Shevchenko, Manivannan Sadhasivam, Rob Herring,
Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Krzysztof Kozlowski, Conor Dooley, Marcel Holtmann,
Luiz Augusto von Dentz, Bartosz Golaszewski, Bartosz Golaszewski,
linux-serial, linux-kernel, linux-kbuild, platform-driver-x86,
linux-pci, devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
Stephan Gerhold, Dmitry Baryshkov, linux-acpi, Hans de Goede,
Bartosz Golaszewski, Luca Ceresoli
In-Reply-To: <CAGXv+5EPA29G-fsH=wWOD8AK6TZFezFhsE0NHPYj_Pt3nT+d_w@mail.gmail.com>
On Wed, Apr 15, 2026 at 04:31:24PM +0800, Chen-Yu Tsai wrote:
> On Tue, Apr 14, 2026 at 8:03 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Apr 14, 2026 at 06:29:02PM +0800, Chen-Yu Tsai wrote:
> > > On Tue, Apr 14, 2026 at 4:28 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Tue, Apr 14, 2026 at 01:03:19PM +0800, Chen-Yu Tsai wrote:
> > > > > On Tue, Apr 14, 2026 at 12:08 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > > > > On Mon, Apr 13, 2026 at 07:33:12PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > On Mon, Apr 13, 2026 at 03:54:59PM +0800, Chen-Yu Tsai wrote:
> > > > > > > > On Thu, Mar 26, 2026 at 01:36:28PM +0530, Manivannan Sadhasivam wrote:
> >
> > ...
> >
> > > > > > > > - Given that this connector actually represents two devices, how do I
> > > > > > > > say I want the BT part to be a wakeup source, but not the WiFi part?
> > > > > > > > Does wakeup-source even work at this point?
> > > > > > >
> > > > > > > You can't use the DT property since the devices are not described in DT
> > > > > > > statically. But you can still use the per-device 'wakeup' sysfs knob to enable
> > > > > > > wakeup.
> > > > >
> > > > > I see. I think not being able to specify generic properties for the devices
> > > > > on the connector is going to be a bit problematic.
> > > >
> > > > This is nature of the open-connectors, especially on the busses that are
> > > > hotpluggable, like PCIe. We never know what is connected there _ahead_.
> > >
> > > I believe what you mean by "hotpluggable" is "user replaceable".
> >
> > From the OS perspective it's the same. From platform perspective
> > there is a difference, granted.
>
> Yes. I just wanted to clarify.
>
> > > > In other words you can't describe in DT something that may not exist.
> > >
> > > But this is actually doable with the PCIe slot representation. The
> > > properties are put in the device node for the slot. If no card is
> > > actually inserted in the slot, then no device is created, and the
> > > device node is left as not associated with anything.
> >
> > But you need to list all devices in the world if you want to support this
>
> Why would I need to? The PCIe slot representation just describes a
> PCIe bridge. Granted this might not be entirely correct, but it's
> what we currently have.
>
> And even then, there are properties like memory-region or wakeup-source
> that are generic and aren't tied to specific devices.
>
> > somehow. Yes, probably many of them (or majority) will be enumerated as is,
> > but some may need an assistance via (dynamic) properties or similar mechanisms.
>
> Even if we wanted to add dynamic properties, there is currently no proper
> device node to attach them to.
>
There are dynamic device nodes that we need to create for hotpluggable devices,
if we need to pass the DT properties to the driver. Like how we do it for PCIe,
serdev. You cannot describe static device nodes in DT for devices attached to
swappable cards like M.2.
> > > It's just that for this new M.2 E-key connector, there aren't separate
> > > nodes for each interface. And the system doesn't associate the device
> > > node with the device, because it's no longer a child node of the
> > > controller or hierarchy, but connected over the OF graph.
> > >
> > > Moving over to the E-key connector representation seems like one step
> > > forward and one step backward in descriptive ability. We gain proper
> > > power sequencing, but lose generic properties.
> >
> > The "key" is property of the connector. Hence if you have an idea what can be
> > common for ALL "key":s, that's probably can be abstracted. Note, I'm not
> > familiar with the connector framework in the Linux kernel, perhaps it's already
> > that kind of abstraction.
>
> I'm not arguing for a even more generic "M.2" connector. The "key" is
> already described in the compatible. I'm saying we should have some way
> of describing the individual interfaces (PCIe, SDIO, USB, UART, I2S, I2C)
> on the connector so further nodes or properties can be attached to them,
> either with overlays or dynamically within the kernel. Right now the
> are only described as individual ports, but we can't actually tie a
> device to a OF graph port.
>
If there are properties that apply to the interfaces, *not devices*, then you
can add them to the relevant endpoint node:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml#n167
> But maybe I'm overthinking the representation part. AFAICT for Qualcomm's
> UART-based BT bit part, Mani just had the driver create a device node
> under the UART (by traversing the OF graph to find the UART). If that's
> the desired way then the connector binding should mention it.
What do you mean by 'connector binding should mention it'? You cannot hardcode
the device node in the connector binding, because it is not part of the
connector binding itself. For example, the UART device node that is created for
the WCN7850, already has a binding:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/bluetooth/qcom,wcn7850-bt.yaml
What the driver does is, just creating that node under the serdev controller as
if the device was described statically in the DT.
For the wakeup property you asked earlier, it depends on the interface. If the
interface supports "in-band" wakeup like USB/SDIO, you can add the property in
the relevant controller node statically itself. For example, with USB XHCI
controller, you would know that the controller will support wakeup or not
statically. So you will add that property to the controller node. Then, if the
user has enabled the wakeup for a specific USB device like keyboard/mouse
through sysfs, then wakeup will just work. Here, we don't need to create the
USB device node at all.
But if the interface supports physical wakeup, like UART_WAKE# in M.2 SDIO Key
E, then based on the presence of that property, you would configure wakeup in
the connector driver itself. But this is not present as of now.
> And that
> works for me. But I think it's messier and also we're missing an
> opportunity to make the M.2 connector a standardized attachment point
> for overlays.
>
I don't envision using overlays for the M.2 connectors. It is not strictly
needed, unless the connector is non-standard.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v2 1/8] dt-bindings: mfd: khadas: Add new compatible for Khadas VIM4 MCU
From: Ronald Claveau @ 2026-04-16 8:25 UTC (permalink / raw)
To: Rob Herring
Cc: Neil Armstrong, Lee Jones, Krzysztof Kozlowski, Conor Dooley,
Andi Shyti, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Beniamino Galvani, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Liam Girdwood, Mark Brown, linux-amlogic, devicetree,
linux-kernel, linux-i2c, linux-arm-kernel, linux-pm
In-Reply-To: <20260415214815.GA602572-robh@kernel.org>
On 4/15/26 11:48 PM, Rob Herring wrote:
> On Fri, Apr 03, 2026 at 06:08:34PM +0200, Ronald Claveau wrote:
>> The Khadas VIM4 MCU register is slightly different
>> from previous boards' MCU.
>> This board also features a switchable power source for its fan.
>>
>> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
>> ---
>> Documentation/devicetree/bindings/mfd/khadas,mcu.yaml | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> index 084960fd5a1fd..67769ef5d58b1 100644
>> --- a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> @@ -18,6 +18,7 @@ properties:
>> compatible:
>> enum:
>> - khadas,mcu # MCU revision is discoverable
>
> The revision is no longer discoverable as was claimed?
>
The firmware revision is still discoverable, and via the same register,
but the VIM4 MCU has a different register layout (eg: no DEVICE_NO
register). The new compatible is needed to describe a different MCU
variant, not a different revision of the same MCU.
I will remove the comment as it is confusing with new boards.
>> + - khadas,vim4-mcu
>>
>> "#cooling-cells": # Only needed for boards having FAN control feature
>> const: 2
>> @@ -25,6 +26,10 @@ properties:
>> reg:
>> maxItems: 1
>>
>> + fan-supply:
>> + description: Phandle to the regulator that powers the fan.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> required:
>> - compatible
>> - reg
>>
>> --
>> 2.49.0
>>
--
Best regards,
Ronald
^ permalink raw reply
* Re: [PATCH v4 02/13] dt-bindings: leds: document Samsung S2M series PMIC RGB LED device
From: Krzysztof Kozlowski @ 2026-04-16 8:23 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
André Draszik, Alexandre Belloni, Jonathan Corbet,
Shuah Khan, Nam Tran, Łukasz Lebiedziński, linux-leds,
devicetree, linux-kernel, linux-pm, linux-samsung-soc, linux-rtc,
linux-doc
In-Reply-To: <DHTWNPSQ06IJ.24A9E1FL1RWER@disroot.org>
On Wed, Apr 15, 2026 at 11:00:16PM +0530, Kaustabh Chakraborty wrote:
> On 2026-04-15 09:03 +02:00, Krzysztof Kozlowski wrote:
> > On Tue, Apr 14, 2026 at 12:02:54PM +0530, Kaustabh Chakraborty wrote:
> >> +description: |
> >> + The Samsung S2M series PMIC RGB LED is a three-channel LED device with
> >> + 8-bit brightness control for each channel, typically used as status
> >> + indicators in mobile phones.
> >> +
> >> + This is a part of device tree bindings for S2M and S5M family of Power
> >> + Management IC (PMIC).
> >> +
> >> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> >> + additional information and example.
> >> +
> >> +allOf:
> >> + - $ref: common.yaml#
> >
> > Rob's comment is still valid:
> > 1. How do you address one of three LEDs in non-RGB case?
> > 2. Where is multi-color?
>
> Yes, multi-color should have been added here.
>
> >
> > And based on this alone without other properties, I say this should be
> > part of top-level schema. Separate node is fine, but no need for
> > separate binding.
>
> BTW, for loading the sub-device driver via platform (as it won't be a
> separate binding) the driver *must* be built-in. Although not related to
> bindings, this seems counter-intuitive. I see the same problem with the
I don't understand that comment. If it has nothing to do with the
binding, what is the problem?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH V10 4/4] thermal: qcom: add support for PMIC5 Gen3 ADC thermal monitoring
From: Jishnu Prakash @ 2026-04-16 8:05 UTC (permalink / raw)
To: Daniel Lezcano
Cc: jic23, robh, krzk+dt, conor+dt, agross, andersson, lumag,
dmitry.baryshkov, konradybcio, daniel.lezcano, sboyd, amitk,
thara.gopinath, lee, rafael, subbaraman.narayanamurthy,
david.collins, anjelique.melendez, kamal.wadhwa, rui.zhang,
lukasz.luba, devicetree, linux-arm-msm, linux-iio, linux-kernel,
linux-pm, cros-qcom-dts-watchers, quic_kotarake, neil.armstrong,
stephan.gerhold
In-Reply-To: <addDTiI8MB2b_AzJ@mai.linaro.org>
Hi Daniel,
On 4/9/2026 11:42 AM, Daniel Lezcano wrote:
> On Fri, Jan 30, 2026 at 05:24:21PM +0530, Jishnu Prakash wrote:
>> Add support for ADC_TM part of PMIC5 Gen3.
>>
>> This is an auxiliary driver under the Gen3 ADC driver, which implements the
>> threshold setting and interrupt generating functionalities of QCOM ADC_TM
>> drivers, used to support thermal trip points.
>>
>> Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
...
>> +
>> +static irqreturn_t adctm5_gen3_isr(int irq, void *dev_id)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = dev_id;
>> + int ret, sdam_num;
>> + u8 tm_status[2];
>> + u8 status, val;
>> +
>> + sdam_num = get_sdam_from_irq(adc_tm5, irq);
>> + if (sdam_num < 0) {
>> + dev_err(adc_tm5->dev, "adc irq %d not associated with an sdam\n",
>> + irq);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + ret = adc5_gen3_read(adc_tm5->dev_data, sdam_num, ADC5_GEN3_STATUS1,
>> + &status, sizeof(status));
>> + if (ret) {
>> + dev_err(adc_tm5->dev, "adc read status1 failed with %d\n", ret);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (status & ADC5_GEN3_STATUS1_CONV_FAULT) {
>> + dev_err_ratelimited(adc_tm5->dev,
>> + "Unexpected conversion fault, status:%#x\n",
>> + status);
>> + val = ADC5_GEN3_CONV_ERR_CLR_REQ;
>> + adc5_gen3_status_clear(adc_tm5->dev_data, sdam_num,
>> + ADC5_GEN3_CONV_ERR_CLR, &val, 1);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + ret = adc5_gen3_read(adc_tm5->dev_data, sdam_num, ADC5_GEN3_TM_HIGH_STS,
>> + tm_status, sizeof(tm_status));
>> + if (ret) {
>> + dev_err(adc_tm5->dev, "adc read TM status failed with %d\n", ret);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (tm_status[0] || tm_status[1])
>> + schedule_work(&adc_tm5->tm_handler_work);
>> +
>> + dev_dbg(adc_tm5->dev, "Interrupt status:%#x, high:%#x, low:%#x\n",
>> + status, tm_status[0], tm_status[1]);
>> +
>> + return IRQ_HANDLED;
>
> This ISR routine should be revisited:
>
> - no error message inside
I'll drop all the error messages, but does that also include the debug print at the end?
In addition, the print for conversion fault is ratelimited and may be useful as it
indicates a possible HW issue, can I keep that?
>
> - use a shared interrupt to split what is handled by the ADC and the
> TM drivers
I'll make the required updates in the main ADC driver and this driver to share the first
SDAM's interrupt.
>
> - do not return IRQ_HANDLED in case of error (cf. irqreturn.h doc)
>
I'll replace IRQ_HANDLED with IRQ_NONE at places where errors are returned.
But in the case of conversion fault, I think returning IRQ_HANDLED may be
more appropriate because we do handle it by clearing the status, to
allow subsequent conversion requests to be sent.
What do you think, is this fine?
> - do not use a dedicated workqueue but the threaded mechanism of the irq
>
I'll make this change.
>> +}
>> +
>> +static int adc5_gen3_tm_status_check(struct adc_tm5_gen3_chip *adc_tm5,
>> + int sdam_index, u8 *tm_status, u8 *buf)
>> +{
>> + int ret;
>> +
>> + ret = adc5_gen3_read(adc_tm5->dev_data, sdam_index, ADC5_GEN3_TM_HIGH_STS,
>> + tm_status, 2);
>> + if (ret) {
>> + dev_err(adc_tm5->dev, "adc read TM status failed with %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = adc5_gen3_status_clear(adc_tm5->dev_data, sdam_index, ADC5_GEN3_TM_HIGH_STS_CLR,
>> + tm_status, 2);
>> + if (ret) {
>> + dev_err(adc_tm5->dev, "adc status clear conv_req failed with %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + ret = adc5_gen3_read(adc_tm5->dev_data, sdam_index, ADC5_GEN3_CH_DATA0(0),
>> + buf, 16);
>> + if (ret)
>> + dev_err(adc_tm5->dev, "adc read data failed with %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void tm_handler_work(struct work_struct *work)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = container_of(work, struct adc_tm5_gen3_chip,
>> + tm_handler_work);
>> + int sdam_index = -1;
>> + u8 tm_status[2] = { };
>> + u8 buf[16] = { };
>> +
>> + for (int i = 0; i < adc_tm5->nchannels; i++) {
>> + struct adc_tm5_gen3_channel_props *chan_prop = &adc_tm5->chan_props[i];
>> + int offset = chan_prop->tm_chan_index;
>> + bool upper_set, lower_set;
>> + int ret, temp;
>> + u16 code;
>> +
>> + scoped_guard(adc5_gen3, adc_tm5) {
>> + if (chan_prop->sdam_index != sdam_index) {
>> + sdam_index = chan_prop->sdam_index;
>> + ret = adc5_gen3_tm_status_check(adc_tm5, sdam_index,
>> + tm_status, buf);
>> + if (ret)
>> + return;
>> + }
>> +
>> + upper_set = ((tm_status[0] & BIT(offset)) && chan_prop->high_thr_en);
>> + lower_set = ((tm_status[1] & BIT(offset)) && chan_prop->low_thr_en);
>> + }
>> +
>> + if (!(upper_set || lower_set))
>> + continue;
>> +
>> + code = get_unaligned_le16(&buf[2 * offset]);
>> + dev_dbg(adc_tm5->dev, "ADC_TM threshold code:%#x\n", code);
>
> Please avoid debug traces when possible
>
>> + ret = adc5_gen3_therm_code_to_temp(adc_tm5->dev,
>> + &chan_prop->common_props,
>> + code, &temp);
>> + if (ret) {
>> + dev_err(adc_tm5->dev,
>> + "Invalid temperature reading, ret = %d, code=%#x\n",
>> + ret, code);
>
> And avoid error traces in the runtime path
Will drop the above prints here and in other similar places if any.
>
>> + continue;
>> + }
>> +
>> + chan_prop->last_temp = temp;
>> + chan_prop->last_temp_set = true;
>> + thermal_zone_device_update(chan_prop->tzd, THERMAL_TRIP_VIOLATED);
>> + }
>> +}
>> +
>> +static int adc_tm5_gen3_get_temp(struct thermal_zone_device *tz, int *temp)
>> +{
>> + struct adc_tm5_gen3_channel_props *prop = thermal_zone_device_priv(tz);
>> + struct adc_tm5_gen3_chip *adc_tm5;
>> +
>> + if (!prop || !prop->chip)
>> + return -EINVAL;
>> +
>> + adc_tm5 = prop->chip;
>> +
>> + if (prop->last_temp_set) {
>> + pr_debug("last_temp: %d\n", prop->last_temp);
>> + prop->last_temp_set = false;
>> + *temp = prop->last_temp;
>> + return 0;
>> + }
>
> Why do you need to do that?
>
> The temperature should reflect the current situation even if the
> reading was triggered by a thermal trip violation.
>
This logic is needed to handle a corner case issue we have seen earlier.
In this case, the ADC_TM threshold violation interrupt gets triggered ,
but when get_temp() is subsequently called by the thermal framework, the
temperature has fluctuated and the value read now lies within the thresholds,
so the thresholds do not get updated by the thermal framework and the violation
interrupts get repeated several times, until there is a get_temp() call
which returns a temperature outside the threshold range.
In order to avoid this issue, when the interrupt handler runs, we find the actual
temperature read in ADC_TM that led to threshold violation by reading the ADC_TM
data registers and we cache it and return it when get_temp() is called in the flow
of thermal_zone_device_update(). Any subsequent calls to get_temp() would
return the actual channel temperature at the time.
This is only done to avoid delaying thermal mitigation due to temperature
fluctuations. Do you think this needs to be changed?
>> +
>> + return adc5_gen3_get_scaled_reading(adc_tm5->dev, &prop->common_props,
>> + temp);
>> +}
>> +
>> +static int adc_tm5_gen3_disable_channel(struct adc_tm5_gen3_channel_props *prop)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = prop->chip;
>> + int ret;
>> + u8 val;
>> +
>> + prop->high_thr_en = false;
>> + prop->low_thr_en = false;
>> +
>> + ret = adc5_gen3_poll_wait_hs(adc_tm5->dev_data, prop->sdam_index);
>> + if (ret)
>> + return ret;
>> +
>> + val = BIT(prop->tm_chan_index);
>> + ret = adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_TM_HIGH_STS_CLR, &val, sizeof(val));
>> + if (ret)
>> + return ret;
>> +
>> + val = MEAS_INT_DISABLE;
>> + ret = adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_TIMER_SEL, &val, sizeof(val));
>> + if (ret)
>> + return ret;
>> +
>> + /* To indicate there is an actual conversion request */
>> + val = ADC5_GEN3_CHAN_CONV_REQ | prop->tm_chan_index;
>> + ret = adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_PERPH_CH, &val, sizeof(val));
>> + if (ret)
>> + return ret;
>> +
>> + val = ADC5_GEN3_CONV_REQ_REQ;
>> + return adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_CONV_REQ, &val, sizeof(val));
>> +}
>> +
>> +#define ADC_TM5_GEN3_CONFIG_REGS 12
>
> Please define at the top of the file
>
>> +static int adc_tm5_gen3_configure(struct adc_tm5_gen3_channel_props *prop,
>> + int low_temp, int high_temp)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = prop->chip;
>> + u8 buf[ADC_TM5_GEN3_CONFIG_REGS];
>> + u8 conv_req;
>> + u16 adc_code;
>> + int ret;
>> +
>> + ret = adc5_gen3_poll_wait_hs(adc_tm5->dev_data, prop->sdam_index);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = adc5_gen3_read(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_SID, buf, sizeof(buf));
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Write SID */
>> + buf[0] = FIELD_PREP(ADC5_GEN3_SID_MASK, prop->common_props.sid);
>> +
>> + /* Select TM channel and indicate there is an actual conversion request */
>> + buf[1] = ADC5_GEN3_CHAN_CONV_REQ | prop->tm_chan_index;
>> +
>> + buf[2] = prop->timer;
>> +
>> + /* Digital param selection */
>> + adc5_gen3_update_dig_param(&prop->common_props, &buf[3]);
>> +
>> + /* Update fast average sample value */
>> + buf[4] &= ~ADC5_GEN3_FAST_AVG_CTL_SAMPLES_MASK;
>> + buf[4] |= prop->common_props.avg_samples | ADC5_GEN3_FAST_AVG_CTL_EN;
>> +
>> + /* Select ADC channel */
>> + buf[5] = prop->common_props.channel;
>> +
>> + /* Select HW settle delay for channel */
>> + buf[6] = FIELD_PREP(ADC5_GEN3_HW_SETTLE_DELAY_MASK,
>> + prop->common_props.hw_settle_time_us);
>> +
>> + /* High temperature corresponds to low voltage threshold */
>> + prop->low_thr_en = (high_temp != INT_MAX);
>> + if (prop->low_thr_en) {
>> + adc_code = qcom_adc_tm5_gen2_temp_res_scale(high_temp);
>> + put_unaligned_le16(adc_code, &buf[8]);
>> + }
>> +
>> + /* Low temperature corresponds to high voltage threshold */
>> + prop->high_thr_en = (low_temp != -INT_MAX);
>> + if (prop->high_thr_en) {
>> + adc_code = qcom_adc_tm5_gen2_temp_res_scale(low_temp);
>> + put_unaligned_le16(adc_code, &buf[10]);
>> + }
>> +
>> + buf[7] = 0;
>> + if (prop->high_thr_en)
>> + buf[7] |= ADC5_GEN3_HIGH_THR_INT_EN;
>> + if (prop->low_thr_en)
>> + buf[7] |= ADC5_GEN3_LOW_THR_INT_EN;
>> +
>> + ret = adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index, ADC5_GEN3_SID,
>> + buf, sizeof(buf));
>> + if (ret < 0)
>> + return ret;
>> +
>> + conv_req = ADC5_GEN3_CONV_REQ_REQ;
>> + return adc5_gen3_write(adc_tm5->dev_data, prop->sdam_index,
>> + ADC5_GEN3_CONV_REQ, &conv_req, sizeof(conv_req));
>> +}
>> +
>> +static int adc_tm5_gen3_set_trip_temp(struct thermal_zone_device *tz,
>> + int low_temp, int high_temp)
>> +{
>> + struct adc_tm5_gen3_channel_props *prop = thermal_zone_device_priv(tz);
>> + struct adc_tm5_gen3_chip *adc_tm5;
>> +
>> + if (!prop || !prop->chip)
>> + return -EINVAL;
>> +
>> + adc_tm5 = prop->chip;
>> +
>> + dev_dbg(adc_tm5->dev, "channel:%s, low_temp(mdegC):%d, high_temp(mdegC):%d\n",
>> + prop->common_props.label, low_temp, high_temp);
>> +
>> + guard(adc5_gen3)(adc_tm5);
>> + if (high_temp == INT_MAX && low_temp == -INT_MAX)
>> + return adc_tm5_gen3_disable_channel(prop);
>
> Why disable the channel instead of returning an errno ?
>
This is the convention we follow in our existing ADC_TM driver at
drivers/thermal/qcom/qcom-spmi-adc-tm5.c. If both upper and lower
thresholds are meant to be disabled, we disable the channel fully
in HW to save some power and it can be enabled later if this API
is called for it with valid thresholds.
Is it considered invalid in the thermal framework to try to disable
both thresholds? Should I both disable the channel and return some
error from here?
>> + return adc_tm5_gen3_configure(prop, low_temp, high_temp);
>> +}
>> +
>> +static const struct thermal_zone_device_ops adc_tm_ops = {
>> + .get_temp = adc_tm5_gen3_get_temp,
>> + .set_trips = adc_tm5_gen3_set_trip_temp,
>> +};
>> +
>> +static int adc_tm5_register_tzd(struct adc_tm5_gen3_chip *adc_tm5)
>> +{
>> + unsigned int i, channel;
>> + struct thermal_zone_device *tzd;
>> + int ret;
>> +
>> + for (i = 0; i < adc_tm5->nchannels; i++) {
>> + channel = ADC5_GEN3_V_CHAN(adc_tm5->chan_props[i].common_props);
>> + tzd = devm_thermal_of_zone_register(adc_tm5->dev, channel,
>> + &adc_tm5->chan_props[i],
>> + &adc_tm_ops);
>> +
>> + if (IS_ERR(tzd)) {
>> + if (PTR_ERR(tzd) == -ENODEV) {
>> + dev_warn(adc_tm5->dev,
>> + "thermal sensor on channel %d is not used\n",
>> + channel);
>> + continue;
>> + }
>> + return dev_err_probe(adc_tm5->dev, PTR_ERR(tzd),
>> + "Error registering TZ zone:%ld for channel:%d\n",
>> + PTR_ERR(tzd), channel);
>> + }
>> + adc_tm5->chan_props[i].tzd = tzd;
>> + ret = devm_thermal_add_hwmon_sysfs(adc_tm5->dev, tzd);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> +static void adc5_gen3_clear_work(void *data)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = data;
>> +
>> + cancel_work_sync(&adc_tm5->tm_handler_work);
>> +}
>> +
>> +static void adc5_gen3_disable(void *data)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = data;
>> + int i;
>> +
>> + guard(adc5_gen3)(adc_tm5);
>> + /* Disable all available TM channels */
>> + for (i = 0; i < adc_tm5->nchannels; i++)
>> + adc_tm5_gen3_disable_channel(&adc_tm5->chan_props[i]);
>> +}
>> +
>> +static void adctm_event_handler(struct auxiliary_device *adev)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5 = auxiliary_get_drvdata(adev);
>> +
>> + schedule_work(&adc_tm5->tm_handler_work);
>> +}
>> +
>> +static int adc_tm5_probe(struct auxiliary_device *aux_dev,
>> + const struct auxiliary_device_id *id)
>> +{
>> + struct adc_tm5_gen3_chip *adc_tm5;
>> + struct tm5_aux_dev_wrapper *aux_dev_wrapper;
>> + struct device *dev = &aux_dev->dev;
>> + int i, ret;
>> +
>> + adc_tm5 = devm_kzalloc(dev, sizeof(*adc_tm5), GFP_KERNEL);
>> + if (!adc_tm5)
>> + return -ENOMEM;
>> +
>> + aux_dev_wrapper = container_of(aux_dev, struct tm5_aux_dev_wrapper,
>> + aux_dev);
>> +
>> + adc_tm5->dev = dev;
>> + adc_tm5->dev_data = aux_dev_wrapper->dev_data;
>> + adc_tm5->nchannels = aux_dev_wrapper->n_tm_channels;
>> + adc_tm5->chan_props = devm_kcalloc(dev, aux_dev_wrapper->n_tm_channels,
>> + sizeof(*adc_tm5->chan_props), GFP_KERNEL);
>> + if (!adc_tm5->chan_props)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < adc_tm5->nchannels; i++) {
>> + adc_tm5->chan_props[i].common_props = aux_dev_wrapper->tm_props[i];
>> + adc_tm5->chan_props[i].timer = MEAS_INT_1S;
>> + adc_tm5->chan_props[i].sdam_index = (i + 1) / 8;
>> + adc_tm5->chan_props[i].tm_chan_index = (i + 1) % 8;
>> + adc_tm5->chan_props[i].chip = adc_tm5;
>> + }
>> +
>> + INIT_WORK(&adc_tm5->tm_handler_work, tm_handler_work);
>
> Why is it needed
I'll drop it as you suggested.
>
>> + /*
>> + * Skipping first SDAM IRQ as it is requested in parent driver.
>> + * If there is a TM violation on that IRQ, the parent driver calls
>> + * the notifier (adctm_event_handler) exposed from this driver to handle it.
>> + */
>> + for (i = 1; i < adc_tm5->dev_data->num_sdams; i++) {
>> + ret = devm_request_threaded_irq(dev,
>> + adc_tm5->dev_data->base[i].irq,
>> + NULL, adctm5_gen3_isr, IRQF_ONESHOT,
>> + adc_tm5->dev_data->base[i].irq_name,
>> + adc_tm5);
>
> The threaded interrupts set the isr in a thread and from the thread
> handling the event, there is a work queue scheduled. Why not use the
> top and bottom halves of the threaded interrupt ? Hopefully you should
> be able to remove the lock.
Yes, I can use the top and bottom halves of the threaded interrupt as you
suggested. But what exactly do you mean by removing the lock?
If you meant the mutex lock used in this driver, we cannot remove that.
This is because the ADC_TM driver needs to write into several registers
shared with the main ADC driver for setting new thresholds, so we
have to share a mutex between the drivers to prevent concurrency issues.
I'll address all your other comments too in the next version of this patch.
Thanks,
Jishnu
>
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + /*
>> + * This drvdata is only used in the function (adctm_event_handler)
>> + * called by parent ADC driver in case of TM violation on the first SDAM.
>> + */
>> + auxiliary_set_drvdata(aux_dev, adc_tm5);
>> +
>> + adc5_gen3_register_tm_event_notifier(dev, adctm_event_handler);
>> +
>> + /*
>> + * This is to cancel any instances of tm_handler_work scheduled by
>> + * TM interrupt, at the time of module removal.
>> + */
>> +
>
> Remove the extra line
>
>> + ret = devm_add_action(dev, adc5_gen3_clear_work, adc_tm5);
>> + if (ret)
>> + return ret;
>> +
>> + ret = adc_tm5_register_tzd(adc_tm5);
>> + if (ret)
>> + return ret;
>> +
>> + /* This is to disable all ADC_TM channels in case of probe failure. */
>> +
>
> Remove the extra line
>
>> + return devm_add_action(dev, adc5_gen3_disable, adc_tm5);
>> +}
>> +
>> +static const struct auxiliary_device_id adctm5_auxiliary_id_table[] = {
>> + { .name = "qcom_spmi_adc5_gen3.adc5_tm_gen3", },
>> + { }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(auxiliary, adctm5_auxiliary_id_table);
>> +
>> +static struct auxiliary_driver adctm5gen3_auxiliary_driver = {
>> + .id_table = adctm5_auxiliary_id_table,
>> + .probe = adc_tm5_probe,
>> +};
>> +
>> +module_auxiliary_driver(adctm5gen3_auxiliary_driver);
>> +
>> +MODULE_DESCRIPTION("SPMI PMIC Thermal Monitor ADC driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_IMPORT_NS("QCOM_SPMI_ADC5_GEN3");
>> --
>> 2.25.1
>>
>
^ permalink raw reply
* Re: [PATCH] PM: hibernate: align default resume swap with image-device checks
From: YoungJun Park @ 2026-04-16 6:38 UTC (permalink / raw)
To: DaeMyung Kang
Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Andrew Morton,
linux-pm, linux-kernel
In-Reply-To: <20260415081213.1732019-1-charsyam@gmail.com>
On Wed, Apr 15, 2026 at 05:12:13PM +0900, DaeMyung Kang wrote:
Hello DaeMyung again :)
> snapshot_open(O_RDONLY) pins the configured default resume swap area for
> hibernation image writes, but it does not fully propagate that choice to
> the image-device checks used by the block layer.
>
> In particular, snapshot_open() uses swsusp_resume_device without
> swsusp_resume_block when pinning the default resume swap area, and it
> leaves snapshot_state.dev unset even after the pin succeeds. As a
> result, the default resume swap selected at open time is not kept
> aligned with is_hibernate_resume_dev() and can still be rejected by the
> block layer in blkdev_write_iter() with -ETXTBSY until user space
> explicitly selects the same area via SNAPSHOT_SET_SWAP_AREA.
>
> Use swsusp_resume_block when pinning the default resume swap area and
> record the device immediately after a successful pin so that the
> hibernation image-device bookkeeping matches the configured resume
> area. Also clear snapshot_state.dev on the open-time error path so it
> never advertises a session that failed to fully open.
>
> uswsusp itself is not affected because it always calls
> SNAPSHOT_SET_SWAP_AREA right after open, which immediately overrides
> snapshot_state.dev and re-pins the swap area. The user-visible change
> is for minimal hibernation user space that relies on resume= and
> resume_offset= alone: such tools no longer have to restate the resume
> area via SNAPSHOT_SET_SWAP_AREA just to satisfy blkdev_write_iter()'s
> IS_SWAPFILE gate or to obtain SWP_HIBERNATION swapoff protection. This
> also matches the intent of the existing "The image device should be
> accessible" comment in snapshot_open(). For the configured default
> resume area, this gives snapshot_open() the same pin and
> recorded-device state that SNAPSHOT_SET_SWAP_AREA would establish for
> that same area, so user space relying on resume= and resume_offset=
> does not need a follow-up SNAPSHOT_SET_SWAP_AREA just to satisfy the
> IS_SWAPFILE gate and obtain swapoff protection.
>
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
So, to summarize: if the resume area and offset are fully specified via
kernel parameters, it should work with hibernation image write properly with just snapshot_open()
without needing an explicit ioctl call, right? And since uswsusp
hibernation tools currently use the ioctl, this hasn't been a visible
issue so far.
> Based on linux-next/master at commit e6efabc0afca ("Add linux-next
> specific files for 20260414").
>
> Tested in QEMU: with swap on a block device pointed to by
> /sys/power/resume, opening /dev/snapshot O_RDONLY and then writing to
> the swap block device returned -ETXTBSY before this change and -ENOSPC
> (i.e. no longer rejected by the IS_SWAPFILE gate) after.
>
> kernel/power/user.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index d0fcfba7ac23..c51b8185de4b 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -69,9 +69,13 @@ static int snapshot_open(struct inode *inode, struct file *filp)
> data = &snapshot_state;
> filp->private_data = data;
> memset(&data->handle, 0, sizeof(struct snapshot_handle));
> + data->dev = 0;
> if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
> /* Hibernating. The image device should be accessible. */
> - data->swap = pin_hibernation_swap_type(swsusp_resume_device, 0);
> + data->swap = pin_hibernation_swap_type(swsusp_resume_device,
> + swsusp_resume_block);
I might miss this situation, just using tool as you explain it.
(cuz it uses ioctl always internally).
Thank you for catching and addressing it.
For checking, it is side effect of my patch,
I check my patch and did some code archaeology to check the history, and it seems this
behavior (on the snapshot_open handler) has been there since the beginning (around v2.6.20).
1. Passing 0 as the offset parameter
2. Not setting data->dev
Both seem to originate from:
commit 915bae9ebe41 ("[PATCH] swsusp: use partition device and offset to
identify swap areas", Dec 6, 2006).
Shortly after, the SNAPSHOT_SET_SWAP_AREA ioctl support was added in
commit 37b2ba12df88 ("[PATCH] swsusp: add ioctl for swap files support",
Dec 6, 2006).
This ioctl patch sets the offset and dev from the user appropriately.
Maybe I am wrong or miss somthing. Please correct me or double-check.
If my understanding is correct, this patch looks like a valid improvement.
It also seems to address an issue that has existed for quite some time.
However, I'm not 100% sure whether the original behavior was intentional,
It would be great to get additional reviews from others.
Best regards,
Youngjun Park.
^ permalink raw reply
* Re: [PATCH v1] cpufreq: pcc: fix use-after-free and double free in _OSC evaluation
From: Zhongqiu Han @ 2026-04-16 6:04 UTC (permalink / raw)
To: Yuho Choi, Rafael J . Wysocki, Viresh Kumar
Cc: linux-pm, linux-kernel, zhongqiu.han
In-Reply-To: <20260415165139.14113-1-dbgh9129@gmail.com>
On 4/16/2026 12:51 AM, Yuho Choi wrote:
> pcc_cpufreq_do_osc() evaluates _OSC twice with the same output buffer.
> The first acpi_evaluate_object() allocates the buffer because output is
> initialized with ACPI_ALLOCATE_BUFFER. Freeing output.pointer before the
> second evaluation leaves output.length stale, so the next call treats
> output as a caller-supplied buffer and performs a use-after-free write
> into the freed memory. The final cleanup path then frees the same
> pointer again, causing a double free.
>
> Keep the first _OSC result alive until the shared cleanup path and route
> the early error exits through out_free. This avoids both the use-after-
> free on the second evaluation and the final double free.
>
> Fixes: 0f1d683fb35d ("[CPUFREQ] Processor Clocking Control interface driver")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---
> drivers/cpufreq/pcc-cpufreq.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> index ac2e90a65f0c4..165826b5d6844 100644
> --- a/drivers/cpufreq/pcc-cpufreq.c
> +++ b/drivers/cpufreq/pcc-cpufreq.c
> @@ -23,6 +23,7 @@
> * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> */
Hi Yuho Choi,
Thanks for the patch.
>
> +#include "acpi/actypes.h"
1.I don't see why this header is needed for this change. Even if it is,
it’s already included by <linux/acpi.h>, right?
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/init.h>
> @@ -351,16 +352,19 @@ static int __init pcc_cpufreq_do_osc(acpi_handle *handle)
> goto out_free;
> }
>
> - kfree(output.pointer);
2.Would it be cleaner to reset the pointer and output.length, and let
acpi_evaluate_object() reallocate the buffer? For example:
kfree(output.pointer);
+ output.pointer = NULL;
+ output.length = ACPI_ALLOCATE_BUFFER;
3.The sashiko.dev pointed out a few pre-existing boundary checking
issues that are outside the scope of this patch.
https://sashiko.dev/#/patchset/20260415165139.14113-1-dbgh9129%40gmail.com
> capabilities[0] = 0x0;
> capabilities[1] = 0x1;
>
> status = acpi_evaluate_object(*handle, "_OSC", &input, &output);
> - if (ACPI_FAILURE(status))
> - return -ENODEV;
> + if (ACPI_FAILURE(status)) {
> + ret = -ENODEV;
> + goto out_free;
> + }
>
> - if (!output.length)
> - return -ENODEV;
> + if (!output.length) {
> + ret = -ENODEV;
> + goto out_free;
> + }
>
> out_obj = output.pointer;
> if (out_obj->type != ACPI_TYPE_BUFFER) {
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2] pmdomain: imx: Make IMX8M/IMX9 BLK_CTRL tristate
From: Daniel Baluta @ 2026-04-16 6:04 UTC (permalink / raw)
To: Zhipeng Wang, Marco Felsch
Cc: ulfh@kernel.org, Frank Li, s.hauer@pengutronix.de,
imx@lists.linux.dev, linux-pm@vger.kernel.org, Xuegang Liu,
Jindong Yue, linux-kernel@vger.kernel.org, kernel@pengutronix.de,
festevam@gmail.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <AMBPR04MB123344C1F802528A10CEA7FADEB252@AMBPR04MB12334.eurprd04.prod.outlook.com>
On 4/14/26 04:59, Zhipeng Wang wrote:
> > On 26-04-13, Zhipeng Wang wrote:
>>> Convert IMX8M_BLK_CTRL and IMX9_BLK_CTRL from bool to tristate to
>>> allow building as loadable modules.
>> Out of curiosity, why do you want to have a PM driver to be buildable as
>> module?
>>
>> Regards,
>> Marco
>>
> Hi Marco,
>
> Thank you for your question.
>
> The primary motivation is to support Google's GKI (Generic Kernel Image)
> requirement for Android devices.
>
> GKI separates the kernel into two parts:
> 1. A unified kernel image (GKI) that is common across all Android devices
> 2. Vendor-specific drivers that must be built as loadable modules
>
> Under the GKI architecture, SoC-specific drivers like IMX8M/IMX9 BLK_CTRL
> cannot be built into the core kernel image. Instead, they must be loadable
> modules that vendors can ship separately. This allows:
>
> - A single kernel binary to support multiple hardware platforms
> - Vendors to update their drivers independently without rebuilding the entire kernel
> - Better compliance with Android's kernel update and security policies
>
Can you please add the below line in the commit message?
> For i.MX8M/i.MX9 devices running Android with GKI kernels, the BLK_CTRL
> drivers need to be loaded as modules during boot. Without tristate support,
> these devices cannot properly initialize their power domains, making them
> non-functional under GKI.
^ permalink raw reply
* Re: [PATCH v2 2/2] riscv: dts: spacemit: Add cpu scaling for K1 SoC
From: Shuwei Wu @ 2026-04-16 5:59 UTC (permalink / raw)
To: Anand Moon
Cc: Rafael J. Wysocki, Viresh Kumar, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Yixun Lan, linux-pm, linux-kernel, linux-riscv,
spacemit, devicetree
In-Reply-To: <CANAwSgSNHO3MNewNzpYbhuj4K3NTdfzDC9KPoUHbFH97P4M_UQ@mail.gmail.com>
On Tue Apr 14, 2026 at 9:25 PM CST, Anand Moon wrote:
> Hi Shuwei,
>
> On Fri, 10 Apr 2026 at 13:30, Shuwei Wu <shuwei.wu@mailbox.org> wrote:
>>
>> Add Operating Performance Points (OPP) tables and CPU clock properties
>> for the two clusters in the SpacemiT K1 SoC.
>>
>> Also assign the CPU power supply (cpu-supply) for the Banana Pi BPI-F3
>> board to fully enable CPU DVFS.
>>
>> Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
>>
>> ---
>> Changes in v2:
>> - Add k1-opp.dtsi with OPP tables for both CPU clusters
>> - Assign CPU supplies and include OPP table for Banana Pi BPI-F3
>> ---
>> arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 35 +++++++-
>> arch/riscv/boot/dts/spacemit/k1-opp.dtsi | 105 ++++++++++++++++++++++++
>> arch/riscv/boot/dts/spacemit/k1.dtsi | 8 ++
>> 3 files changed, 147 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> index 444c3b1e6f44..3780593f610d 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> @@ -5,6 +5,7 @@
>>
>> #include "k1.dtsi"
>> #include "k1-pinctrl.dtsi"
>> +#include "k1-opp.dtsi"
>>
>> / {
>> model = "Banana Pi BPI-F3";
>> @@ -86,6 +87,38 @@ &combo_phy {
>> status = "okay";
>> };
>>
>> +&cpu_0 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_1 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_2 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_3 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_4 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_5 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_6 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> +&cpu_7 {
>> + cpu-supply = <&buck1_3v45>;
>> +};
>> +
>> &emmc {
>> bus-width = <8>;
>> mmc-hs400-1_8v;
>> @@ -201,7 +234,7 @@ pmic@41 {
>> dldoin2-supply = <&buck5>;
>>
>> regulators {
>> - buck1 {
>> + buck1_3v45: buck1 {
>> regulator-min-microvolt = <500000>;
>> regulator-max-microvolt = <3450000>;
>> regulator-ramp-delay = <5000>;
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-opp.dtsi b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
>> new file mode 100644
>> index 000000000000..768ae390686d
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/spacemit/k1-opp.dtsi
>> @@ -0,0 +1,105 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +/ {
>> + cluster0_opp_table: opp-table-cluster0 {
>> + compatible = "operating-points-v2";
>> + opp-shared;
>> +
>> + opp-614400000 {
>> + opp-hz = /bits/ 64 <614400000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-819000000 {
>> + opp-hz = /bits/ 64 <819000000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1000000000 {
>> + opp-hz = /bits/ 64 <1000000000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1228800000 {
>> + opp-hz = /bits/ 64 <1228800000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1600000000 {
>> + opp-hz = /bits/ 64 <1600000000>;
>> + opp-microvolt = <1050000>;
>> + clock-latency-ns = <200000>;
>> + };
>> + };
>> +
>> + cluster1_opp_table: opp-table-cluster1 {
>> + compatible = "operating-points-v2";
>> + opp-shared;
>> +
>> + opp-614400000 {
>> + opp-hz = /bits/ 64 <614400000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-819000000 {
>> + opp-hz = /bits/ 64 <819000000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1000000000 {
>> + opp-hz = /bits/ 64 <1000000000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1228800000 {
>> + opp-hz = /bits/ 64 <1228800000>;
>> + opp-microvolt = <950000>;
>> + clock-latency-ns = <200000>;
>> + };
>> +
>> + opp-1600000000 {
>> + opp-hz = /bits/ 64 <1600000000>;
>> + opp-microvolt = <1050000>;
>> + clock-latency-ns = <200000>;
>> + };
>> + };
>> +};
>> +
>> +&cpu_0 {
>> + operating-points-v2 = <&cluster0_opp_table>;
>> +};
>> +
>> +&cpu_1 {
>> + operating-points-v2 = <&cluster0_opp_table>;
>> +};
>> +
>> +&cpu_2 {
>> + operating-points-v2 = <&cluster0_opp_table>;
>> +};
>> +
>> +&cpu_3 {
>> + operating-points-v2 = <&cluster0_opp_table>;
>> +};
>> +
>> +&cpu_4 {
>> + operating-points-v2 = <&cluster1_opp_table>;
>> +};
>> +
>> +&cpu_5 {
>> + operating-points-v2 = <&cluster1_opp_table>;
>> +};
>> +
>> +&cpu_6 {
>> + operating-points-v2 = <&cluster1_opp_table>;
>> +};
>> +
>> +&cpu_7 {
>> + operating-points-v2 = <&cluster1_opp_table>;
>> +};
>> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
>> index 529ec68e9c23..bdd109b81730 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
>> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
>> @@ -54,6 +54,7 @@ cpu_0: cpu@0 {
>> compatible = "spacemit,x60", "riscv";
>> device_type = "cpu";
>> reg = <0>;
>> + clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
>> riscv,isa = "rv64imafdcbv_zicbom_zicbop_zicboz_zicntr_zicond_zicsr_zifencei_zihintpause_zihpm_zfh_zba_zbb_zbc_zbs_zkt_zvfh_zvkt_sscofpmf_sstc_svinval_svnapot_svpbmt";
>> riscv,isa-base = "rv64i";
>> riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "v", "zicbom",
>> @@ -84,6 +85,7 @@ cpu_1: cpu@1 {
>> compatible = "spacemit,x60", "riscv";
>> device_type = "cpu";
>> reg = <1>;
>> + clocks = <&syscon_apmu CLK_CPU_C0_CORE>;
>
> Based on the Spacemit kernel source, the k1-x_opp_table.dtsi file
> defines several additional clocks for the Operating Performance Points
> (OPP) table:
>
> clocks = <&ccu CLK_CPU_C0_ACE>, <&ccu CLK_CPU_C1_ACE>, <&ccu CLK_CPU_C0_TCM>,
> <&ccu CLK_CCI550>, <&ccu CLK_PLL3>, <&ccu
> CLK_CPU_C0_HI>, <&ccu CLK_CPU_C1_HI>;
> clock-names = "ace0","ace1","tcm","cci","pll3", "c0hi", "c1hi";
>
> These hardware clocks are also explicitly registered in the APMU clock driver
> via the k1_ccu_apmu_hws array, confirming their availability for frequency
> and voltage scaling on the K1-X SoC.
>
> static struct clk_hw *k1_ccu_apmu_hws[] = {
> [CLK_CCI550] = &cci550_clk.common.hw,
> [CLK_CPU_C0_HI] = &cpu_c0_hi_clk.common.hw,
> [CLK_CPU_C0_CORE] = &cpu_c0_core_clk.common.hw,
> [CLK_CPU_C0_ACE] = &cpu_c0_ace_clk.common.hw,
> [CLK_CPU_C0_TCM] = &cpu_c0_tcm_clk.common.hw,
> [CLK_CPU_C1_HI] = &cpu_c1_hi_clk.common.hw,
> [CLK_CPU_C1_CORE] = &cpu_c1_core_clk.common.hw,
> [CLK_CPU_C1_ACE] = &cpu_c1_ace_clk.common.hw,
>
> Yes, it is possible to add these clocks for DVFS to work correctly,
> provided they are managed by the appropriate driver and declared in
> the Device Tree (DT).
>
> Thanks
> -Anand
Thanks for your review and for pointing this out.
Regarding the clocks you mentioned, I'd like to clarify their roles based on
the K1 datasheet. Taking Cluster 0 as an example, c0_core_clk is the primary
clock for the cluster. c0_ace_clk and c0_tcm_clk are children derived from it,
defaulting to half the frequency of their parent core clock, while c0_hi_clk
represents the high-speed path selection.
Cluster 1 follows the same structure.
Based on the official SpacemiT Bianbu OS source, the spacemit-cpufreq.c driver
mainly performs the following tasks:
1. Sets the CCI550 clock frequency to 614MHz.
2. Sets the clock frequencies of c0_ace_clk, c1_ace1_clk, and c0_tcm_clk to half
the frequency of their parent clock.
3. For the 1.6GHz OPP, it sets the PLL3 frequency to 3.2GHz and the
c0_hi_clk/c1_hi_clk frequencies to 1.6GHz.
I booted with the manufacturer's OpenWRT image and used debugfs to confirm that
the clock states are exactly as described above.
At 1.6GHz:
Clock Source & Tree Rate (Hz) HW Enable Consumer
---------------------------------------------------------------------------
pll3 3,200,000,000 Y deviceless
└─ pll3_d2 1,600,000,000 Y deviceless
├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
│ └─ cpu_c1_pclk 1,600,000,000 Y cpu0
│ └─ cpu_c1_ace_clk 800,000,000 Y deviceless
└─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
└─ cpu_c0_core_clk 1,600,000,000 Y cpu0
├─ cpu_c0_tcm_clk 800,000,000 Y deviceless
└─ cpu_c0_ace_clk 800,000,000 Y deviceless
pll1_2457p6_vco 2,457,600,000 Y deviceless
└─ pll1_d4 614,400,000 Y deviceless
└─ pll1_d4_614p4 614,400,000 Y deviceless
└─ cci550_clk 614,400,000 Y deviceless
At 1.228GHz:
Clock Source & Tree Rate (Hz) HW Enable Consumer
---------------------------------------------------------------------------
pll1_2457p6_vco 2,457,600,000 Y deviceless
└─ pll1_d2 1,228,800,000 Y deviceless
└─ pll1_d2_1228p8 1,228,800,000 Y deviceless
├─ cpu_c0_core_clk 1,228,800,000 Y cpu0
│ ├─ cpu_c0_tcm_clk 614,400,000 Y deviceless
│ └─ cpu_c0_ace_clk 614,400,000 Y deviceless
└─ cpu_c1_pclk 1,228,800,000 Y cpu0
└─ cpu_c1_ace_clk 614,400,000 Y deviceless
└─ pll1_d4 614,400,000 Y deviceless
└─ pll1_d4_614p4 614,400,000 Y deviceless
└─ cci550_clk 614,400,000 Y deviceless
pll3 3,200,000,000 Y deviceless
└─ pll3_d2 1,600,000,000 Y deviceless
├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
└─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
└─ pll3_d3 1,066,666,666 Y deviceless
Regarding the necessity of listing these clocks in the DT, my analysis is as follows:
1. For CCI550, I did not find a clear definition of this clock's specific role
in the SoC datasheet. Although the vendor kernel increases its frequency,
my benchmarks show that maintaining the mainline default (245.76MHz) has a
negligible impact on CPU performance.
2. For ACE and TCM clocks, they function as synchronous children of the core
clock with a default divide-by-2 ratio. Since they scale automatically relative
to c0_core_clk/c1_core_clk and no other peripherals depend on them, they do not
require manual management in the OPP table.
3. For the high-speed path, the underlying clock controller logic already handles
the parent MUX switching and PLL3 scaling automatically when clk_set_rate()
is called on the core clock.
I have verified this by checking the hardware state in the mainline kernel.
The clock tree matches the vendor kernel's configuration:
At 1.6GHz:
Clock Source & Tree Rate (Hz) HW Enable Consumer
---------------------------------------------------------------------------
pll3 3,200,000,000 Y deviceless
└─ pll3_d2 1,600,000,000 Y deviceless
├─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
│ └─ cpu_c1_core_clk 1,600,000,000 Y cpu4
│ └─ cpu_c1_ace_clk 800,000,000 Y deviceless
└─ cpu_c0_hi_clk 1,600,000,000 Y deviceless
└─ cpu_c0_core_clk 1,600,000,000 Y cpu0
├─ cpu_c0_tcm_clk 800,000,000 Y deviceless
└─ cpu_c0_ace_clk 800,000,000 Y deviceless
pll1 2,457,600,000 Y deviceless
└─ pll1_d5 491,520,000 Y deviceless
└─ pll1_d5_491p52 491,520,000 Y deviceless
└─ cci550_clk 245,760,000 Y deviceless
At 1.228GHz:
Clock Source & Tree Rate (Hz) HW Enable Consumer
---------------------------------------------------------------------------
pll1 2,457,600,000 Y deviceless
├─ pll1_d5 491,520,000 Y deviceless
│ └─ pll1_d5_491p52 491,520,000 Y deviceless
│ └─ cci550_clk 245,760,000 Y deviceless
└─ pll1_d2 1,228,800,000 Y deviceless
└─ pll1_d2_1228p8 1,228,800,000 Y deviceless
└─ cpu_c0_core_clk 1,228,800,000 Y cpu0
├─ cpu_c0_tcm_clk 614,400,000 Y deviceless
└─ cpu_c0_ace_clk 614,400,000 Y deviceless
pll3 3,200,000,000 Y deviceless
└─ pll3_d2 1,600,000,000 Y deviceless
└─ cpu_c1_hi_clk 1,600,000,000 Y deviceless
└─ cpu_c1_core_clk 1,600,000,000 Y cpu4
└─ cpu_c1_ace_clk 800,000,000 Y deviceless
Performance benchmarks also confirm that the current configuration is sufficient:
Benchmark (AWK computation): time awk 'BEGIN{for(i=0;i<10000000;i++) sum+=i}'
----------------------------------------------------------------------------
Frequency | Mainline Linux (s) | OpenWrt (s)
(kHz) | Real (Total) | User (CPU) | Real (Total) | User (CPU) )
-------------+---------------+---------------+---------------+--------------
1,600,000 | 1.82s | 1.81s | 1.73s | 1.73s
1,228,800 | 2.34s | 2.33s | 2.26s | 2.26s
1,000,000 | 2.94s | 2.86s | 2.78s | 2.78s
819,000 | 3.54s | 3.53s | 3.39s | 3.39s
614,400 | 4.73s | 4.71s | 4.51s | 4.51s
----------------------------------------------------------------------------
In summary, because the clock controller correctly handles the internal dividers
and parent switching, declaring only the primary core clock for each CPU node is
sufficient for functional DVFS.
--
Best regards,
Shuwei Wu
^ permalink raw reply
* Re: [PATCH v3] pmdomain: imx: Make IMX8M/IMX9 BLK_CTRL tristate
From: Daniel Baluta @ 2026-04-16 6:01 UTC (permalink / raw)
To: Zhipeng Wang, ulfh, Frank.Li, s.hauer
Cc: kernel, festevam, linux-pm, imx, linux-arm-kernel, linux-kernel,
xuegang.liu, jindong.yue
In-Reply-To: <20260416015605.3536244-1-zhipeng.wang_1@nxp.com>
On 4/16/26 04:56, Zhipeng Wang wrote:
> Convert IMX8M_BLK_CTRL and IMX9_BLK_CTRL from bool to tristate
> to allow building as loadable modules.
>
> Add prompt strings to make these options visible and configurable
> in menuconfig, keeping them enabled by default on appropriate platforms.
>
> Also remove the IMX_GPCV2_PM_DOMAINS dependency from IMX9_BLK_CTRL.
> This dependency was incorrect from the beginning because i.MX93 uses a
> different power domain architecture compared to i.MX8M series:
>
> - i.MX8M uses GPCv2 (General Power Controller v2) for power domain
> management, hence IMX8M_BLK_CTRL correctly depends on it.
>
> - i.MX93 uses BLK_CTRL directly without GPCv2. The hardware doesn't
> have GPCv2 at all.
>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> ---
Please always add a change log here to help reviewers.
Change since v2:
* fixed typo reported by Frank
^ permalink raw reply
* RE: [PATCH v2] pmdomain: imx: Make IMX8M/IMX9 BLK_CTRL tristate
From: Zhipeng Wang @ 2026-04-16 1:59 UTC (permalink / raw)
To: Frank Li
Cc: ulfh@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de,
festevam@gmail.com, linux-pm@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Xuegang Liu, Jindong Yue
In-Reply-To: <ad83MtdMP4rci3Ya@lizhi-Precision-Tower-5810>
> On Mon, Apr 13, 2026 at 02:30:49PM +0900, Zhipeng Wang wrote:
> > Convert IMX8M_BLK_CTRL and IMX9_BLK_CTRL from bool to tristate to
> > allow building as loadable modules.
> >
> > Add prompt strings to make these options visible and configurable in
> > menuconfig, keeping them enabled by default on appropriate platforms.
> >
> > Also remove the IMX_GPCV2_PM_DOMAINS dependency from
> IMX9_BLK_CTRL.
> > This dependency was incorrect from the beginning - i.MX93 uses a
>
> s/-/because
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
Hi Frank,
Thank you! v3 sent.
BRs,
Zhipeng
> > different power domain architecture compared to i.MX8M series:
> >
> > - i.MX8M uses GPCv2 (General Power Controller v2) for power domain
> > management, hence IMX8M_BLK_CTRL correctly depends on it.
> >
> > - i.MX93 uses BLK_CTRL directly without GPCv2. The hardware doesn't
> > have GPCv2 at all.
> >
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > ---
> > drivers/pmdomain/imx/Kconfig | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pmdomain/imx/Kconfig
> > b/drivers/pmdomain/imx/Kconfig index 00203615c65e..9168d183b0c5
> 100644
> > --- a/drivers/pmdomain/imx/Kconfig
> > +++ b/drivers/pmdomain/imx/Kconfig
> > @@ -10,15 +10,18 @@ config IMX_GPCV2_PM_DOMAINS
> > default y if SOC_IMX7D
> >
> > config IMX8M_BLK_CTRL
> > - bool
> > - default SOC_IMX8M && IMX_GPCV2_PM_DOMAINS
> > + tristate "i.MX8M BLK CTRL driver"
> > + depends on SOC_IMX8M
> > + depends on IMX_GPCV2_PM_DOMAINS
> > depends on PM_GENERIC_DOMAINS
> > depends on COMMON_CLK
> > + default y
> >
> > config IMX9_BLK_CTRL
> > - bool
> > - default SOC_IMX9 && IMX_GPCV2_PM_DOMAINS
> > + tristate "i.MX93 BLK CTRL driver"
> > + depends on SOC_IMX9
> > depends on PM_GENERIC_DOMAINS
> > + default y
> >
> > config IMX_SCU_PD
> > bool "IMX SCU Power Domain driver"
> > --
> > 2.34.1
> >
^ permalink raw reply
* [PATCH v3] pmdomain: imx: Make IMX8M/IMX9 BLK_CTRL tristate
From: Zhipeng Wang @ 2026-04-16 1:56 UTC (permalink / raw)
To: ulfh, Frank.Li, s.hauer
Cc: kernel, festevam, linux-pm, imx, linux-arm-kernel, linux-kernel,
xuegang.liu, jindong.yue
Convert IMX8M_BLK_CTRL and IMX9_BLK_CTRL from bool to tristate
to allow building as loadable modules.
Add prompt strings to make these options visible and configurable
in menuconfig, keeping them enabled by default on appropriate platforms.
Also remove the IMX_GPCV2_PM_DOMAINS dependency from IMX9_BLK_CTRL.
This dependency was incorrect from the beginning because i.MX93 uses a
different power domain architecture compared to i.MX8M series:
- i.MX8M uses GPCv2 (General Power Controller v2) for power domain
management, hence IMX8M_BLK_CTRL correctly depends on it.
- i.MX93 uses BLK_CTRL directly without GPCv2. The hardware doesn't
have GPCv2 at all.
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
drivers/pmdomain/imx/Kconfig | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/pmdomain/imx/Kconfig b/drivers/pmdomain/imx/Kconfig
index 00203615c65e..9168d183b0c5 100644
--- a/drivers/pmdomain/imx/Kconfig
+++ b/drivers/pmdomain/imx/Kconfig
@@ -10,15 +10,18 @@ config IMX_GPCV2_PM_DOMAINS
default y if SOC_IMX7D
config IMX8M_BLK_CTRL
- bool
- default SOC_IMX8M && IMX_GPCV2_PM_DOMAINS
+ tristate "i.MX8M BLK CTRL driver"
+ depends on SOC_IMX8M
+ depends on IMX_GPCV2_PM_DOMAINS
depends on PM_GENERIC_DOMAINS
depends on COMMON_CLK
+ default y
config IMX9_BLK_CTRL
- bool
- default SOC_IMX9 && IMX_GPCV2_PM_DOMAINS
+ tristate "i.MX93 BLK CTRL driver"
+ depends on SOC_IMX9
depends on PM_GENERIC_DOMAINS
+ default y
config IMX_SCU_PD
bool "IMX SCU Power Domain driver"
--
2.34.1
^ permalink raw reply related
* Re: [GIT PULL] pmdomain updates for v7.1
From: pr-tracker-bot @ 2026-04-15 22:00 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Linus, linux-pm, linux-kernel, Ulf Hansson, linux-arm-kernel
In-Reply-To: <20260414103826.161076-1-ulf.hansson@linaro.org>
The pull request you sent on Tue, 14 Apr 2026 12:38:21 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git tags/pmdomain-v7.1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e41a25c53f96abe40edc5db1626d37a518852d84
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2 2/8] dt-bindings: i2c: amlogic: Add compatible for T7 SOC
From: Rob Herring (Arm) @ 2026-04-15 21:48 UTC (permalink / raw)
To: Ronald Claveau
Cc: Jerome Brunet, Neil Armstrong, linux-i2c, Kevin Hilman, linux-pm,
Lukasz Luba, linux-amlogic, linux-kernel, Zhang Rui, Lee Jones,
devicetree, Conor Dooley, Andi Shyti, Daniel Lezcano,
Martin Blumenstingl, Beniamino Galvani, Krzysztof Kozlowski,
Liam Girdwood, Mark Brown, linux-arm-kernel, Rafael J. Wysocki
In-Reply-To: <20260403-add-mcu-fan-khadas-vim4-v2-2-70536b22439a@aliel.fr>
On Fri, 03 Apr 2026 18:08:35 +0200, Ronald Claveau wrote:
> Add the T7 SOC compatible which fallback to AXG compatible.
>
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
> .../devicetree/bindings/i2c/amlogic,meson6-i2c.yaml | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox