* [PATCH 0/3] soc: apple: Add "PMGR misc" power controls driver
@ 2026-07-02 12:06 Sasha Finkelstein
2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Sasha Finkelstein @ 2026-07-02 12:06 UTC (permalink / raw)
To: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: asahi, linux-arm-kernel, devicetree, linux-kernel,
Sasha Finkelstein, Hector Martin
Certain Apple SoCs have additional power state controls that are using
a separate "misc" mmio interface. Currently this includes the fabric
and memory controllers on pro/max/ultra SoCs.
This series adds a driver to put those in a low power state when the
machine enters sleep. The power savings are SoC-dependent and are
around 1W.
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
Hector Martin (1):
soc: apple: Add driver for Apple PMGR misc controls
Sasha Finkelstein (2):
dt-bindings: soc: apple: Add bindings for apple PMGR misc controls
arm64: dts: apple: Add pmgr-misc nodes to t60xx
Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml | 45 +++++++++++++++++++++++++++
MAINTAINERS | 1 +
arch/arm64/boot/dts/apple/t600x-die0.dtsi | 7 +++++
arch/arm64/boot/dts/apple/t602x-die0.dtsi | 7 +++++
drivers/soc/apple/Kconfig | 10 ++++++
drivers/soc/apple/Makefile | 2 ++
drivers/soc/apple/apple-pmgr-misc.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 249 insertions(+)
---
base-commit: 59574e5978abfc2ce1c194ea0200eb6fc718d8ce
change-id: 20260702-pmgr-misc-ae1cbd5bc2c7
Best regards,
--
Sasha Finkelstein <k@chaosmail.tech>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls 2026-07-02 12:06 [PATCH 0/3] soc: apple: Add "PMGR misc" power controls driver Sasha Finkelstein @ 2026-07-02 12:06 ` Sasha Finkelstein 2026-07-02 19:18 ` Conor Dooley 2026-07-03 9:52 ` Krzysztof Kozlowski 2026-07-02 12:06 ` [PATCH 2/3] soc: apple: Add driver for Apple " Sasha Finkelstein 2026-07-02 12:06 ` [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx Sasha Finkelstein 2 siblings, 2 replies; 9+ messages in thread From: Sasha Finkelstein @ 2026-07-02 12:06 UTC (permalink / raw) To: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, Sasha Finkelstein Certain Apple SoCs include additional PMGR power states that are controlled via a different "misc" control block. On existing SoCs, this includes the fabric and memory controller state. Signed-off-by: Sasha Finkelstein <k@chaosmail.tech> --- Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml | 45 +++++++++++++++++++++++++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 46 insertions(+) diff --git a/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml b/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml new file mode 100644 index 000000000000..30abedc67fa4 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/apple/apple,t6000-pmgr-misc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Apple SoC PMGR Misc Power States + +maintainers: + - Sasha Finkelstein <k@chaosmail.tech> + +description: | + Certain Apple SoCs include additional PMGR power states that are controlled + via a different "misc" control block. On existing SoCs, this includes the + fabric and memory controller state. + +properties: + compatible: + enum: + - apple,t6000-pmgr-misc + - apple,t6020-pmgr-misc + + reg: + maxItems: 2 + + reg-names: + items: + - const: fabric-ps + - const: dcs-ps + +required: + - compatible + - reg + - reg-names + +additionalProperties: false + +examples: + - | + pmgr_misc: power-management@8e20c000 { + compatible = "apple,t6000-pmgr-misc"; + reg = <0x8e20c000 0x400>, + <0x8e20c800 0x400>; + reg-names = "fabric-ps", "dcs-ps"; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 15011f5752a9..5a29bb86499f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2607,6 +2607,7 @@ F: Documentation/devicetree/bindings/power/apple* F: Documentation/devicetree/bindings/power/reset/apple,smc-reboot.yaml F: Documentation/devicetree/bindings/pwm/apple,s5l-fpwm.yaml F: Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml +F: Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml F: Documentation/devicetree/bindings/spi/apple,spi.yaml F: Documentation/devicetree/bindings/spmi/apple,spmi.yaml F: Documentation/devicetree/bindings/usb/apple,dwc3.yaml -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls 2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein @ 2026-07-02 19:18 ` Conor Dooley 2026-07-02 19:35 ` Sasha Finkelstein 2026-07-03 9:52 ` Krzysztof Kozlowski 1 sibling, 1 reply; 9+ messages in thread From: Conor Dooley @ 2026-07-02 19:18 UTC (permalink / raw) To: Sasha Finkelstein Cc: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 3293 bytes --] On Thu, Jul 02, 2026 at 02:06:26PM +0200, Sasha Finkelstein wrote: > Certain Apple SoCs include additional PMGR power states that are > controlled via a different "misc" control block. On existing SoCs, this > includes the fabric and memory controller state. > > Signed-off-by: Sasha Finkelstein <k@chaosmail.tech> > --- > Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml | 45 +++++++++++++++++++++++++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 46 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml b/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml > new file mode 100644 > index 000000000000..30abedc67fa4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml > @@ -0,0 +1,45 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/apple/apple,t6000-pmgr-misc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple SoC PMGR Misc Power States > + > +maintainers: > + - Sasha Finkelstein <k@chaosmail.tech> > + > +description: | > + Certain Apple SoCs include additional PMGR power states that are controlled > + via a different "misc" control block. On existing SoCs, this includes the > + fabric and memory controller state. Are t6000 and t6020 the "certain Apple SoCs"? I assume so. The bit about "existing SoCs" is confusing in a binding (although it would be ok in the commit message), because you run into language semantics that imply that this sentence is about devices to the ones you're addressing here. I think you can just do s/. On existing SoCs//. And add an s to the final "state" to make it "states". > + > +properties: > + compatible: > + enum: > + - apple,t6000-pmgr-misc > + - apple,t6020-pmgr-misc > + > + reg: > + maxItems: 2 > + > + reg-names: > + items: > + - const: fabric-ps > + - const: dcs-ps "dcs" means memory controller then? > + > +required: > + - compatible > + - reg > + - reg-names > + > +additionalProperties: false > + > +examples: > + - | > + pmgr_misc: power-management@8e20c000 { If there's a new version, drop the unused label here please. > + compatible = "apple,t6000-pmgr-misc"; > + reg = <0x8e20c000 0x400>, > + <0x8e20c800 0x400>; > + reg-names = "fabric-ps", "dcs-ps"; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 15011f5752a9..5a29bb86499f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2607,6 +2607,7 @@ F: Documentation/devicetree/bindings/power/apple* > F: Documentation/devicetree/bindings/power/reset/apple,smc-reboot.yaml > F: Documentation/devicetree/bindings/pwm/apple,s5l-fpwm.yaml > F: Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml > +F: Documentation/devicetree/bindings/soc/apple/apple,t6000-pmgr-misc.yaml > F: Documentation/devicetree/bindings/spi/apple,spi.yaml > F: Documentation/devicetree/bindings/spmi/apple,spmi.yaml > F: Documentation/devicetree/bindings/usb/apple,dwc3.yaml > > -- > 2.55.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls 2026-07-02 19:18 ` Conor Dooley @ 2026-07-02 19:35 ` Sasha Finkelstein 0 siblings, 0 replies; 9+ messages in thread From: Sasha Finkelstein @ 2026-07-02 19:35 UTC (permalink / raw) To: Conor Dooley Cc: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel, devicetree, linux-kernel On Jul 2, 2026, at 21:18, Conor Dooley <conor@kernel.org> wrote: > Are t6000 and t6020 the "certain Apple SoCs"? I assume so. The bit about > "existing SoCs" is confusing in a binding (although it would be ok in the > commit message), because you run into language semantics that imply that > this sentence is about devices to the ones you're addressing here. I > think you can just do s/. On existing SoCs//. And add an s to the final > "state" to make it "states". t600{0,1,2}, t602{0,1,2}, t603{1,2,4} (no upstream DT for those yet) It is kind of a long list, with most t6xxx ones on it. > "dcs" means memory controller then? It is memory controller related, unclear if this covers the entire controller or a subset of it. The name comes from Apple device trees and macos kernel logs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls 2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein 2026-07-02 19:18 ` Conor Dooley @ 2026-07-03 9:52 ` Krzysztof Kozlowski 1 sibling, 0 replies; 9+ messages in thread From: Krzysztof Kozlowski @ 2026-07-03 9:52 UTC (permalink / raw) To: Sasha Finkelstein Cc: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel, devicetree, linux-kernel On Thu, Jul 02, 2026 at 02:06:26PM +0200, Sasha Finkelstein wrote: > Certain Apple SoCs include additional PMGR power states that are > controlled via a different "misc" control block. On existing SoCs, this > includes the fabric and memory controller state. > > Signed-off-by: Sasha Finkelstein <k@chaosmail.tech> Just a nit on top of Conor's review, if there is a new version: A nit, subject: drop second/last, redundant "bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23 Best regards, Krzysztof ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] soc: apple: Add driver for Apple PMGR misc controls 2026-07-02 12:06 [PATCH 0/3] soc: apple: Add "PMGR misc" power controls driver Sasha Finkelstein 2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein @ 2026-07-02 12:06 ` Sasha Finkelstein 2026-07-02 12:23 ` sashiko-bot 2026-07-02 12:06 ` [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx Sasha Finkelstein 2 siblings, 1 reply; 9+ messages in thread From: Sasha Finkelstein @ 2026-07-02 12:06 UTC (permalink / raw) To: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, Sasha Finkelstein, Hector Martin From: Hector Martin <marcan@marcan.st> Apple SoCs have PMGR blocks that control a bunch of power-related features. Besides the existing device power state controls (which are very uniform and handled by apple-pmgr-pwrstate), we also need to manage more random registers such as SoC-wide fabric and memory controller power states, which have a different interface. Add a driver for these kitchen sink controls. Right now it implements fabric and memory controller power state switching on system standby/s2idle, which saves about 1W of power or so on t60xx platforms. Signed-off-by: Hector Martin <marcan@marcan.st> Co-developed-by: Sasha Finkelstein <k@chaosmail.tech> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech> --- drivers/soc/apple/Kconfig | 10 ++++++++ drivers/soc/apple/Makefile | 2 ++ drivers/soc/apple/apple-pmgr-misc.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+) diff --git a/drivers/soc/apple/Kconfig b/drivers/soc/apple/Kconfig index d0ff32182a2b..cad847c90df8 100644 --- a/drivers/soc/apple/Kconfig +++ b/drivers/soc/apple/Kconfig @@ -16,6 +16,16 @@ config APPLE_MAILBOX Say Y here if you have an Apple SoC. +config APPLE_PMGR_MISC + bool "Apple SoC PMGR miscellaneous support" + depends on PM + help + The PMGR block in Apple SoCs provides high-level power state + controls for SoC devices. This driver manages miscellaneous + power controls. + + Say 'y' here if you have a Pro/Max/Ultra Apple SoC. + config APPLE_RTKIT tristate "Apple RTKit co-processor IPC protocol" depends on APPLE_MAILBOX diff --git a/drivers/soc/apple/Makefile b/drivers/soc/apple/Makefile index 0b85ab61aefe..0481f7f43b06 100644 --- a/drivers/soc/apple/Makefile +++ b/drivers/soc/apple/Makefile @@ -3,6 +3,8 @@ obj-$(CONFIG_APPLE_MAILBOX) += apple-mailbox.o apple-mailbox-y = mailbox.o +obj-$(CONFIG_APPLE_PMGR_MISC) += apple-pmgr-misc.o + obj-$(CONFIG_APPLE_RTKIT) += apple-rtkit.o apple-rtkit-y = rtkit.o rtkit-crashlog.o diff --git a/drivers/soc/apple/apple-pmgr-misc.c b/drivers/soc/apple/apple-pmgr-misc.c new file mode 100644 index 000000000000..d487a87ecddb --- /dev/null +++ b/drivers/soc/apple/apple-pmgr-misc.c @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: GPL-2.0-only OR MIT +/* + * Apple SoC PMGR device power state driver + * + * Copyright The Asahi Linux Contributors + */ + +#include <linux/bitops.h> +#include <linux/bitfield.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/module.h> + +#define APPLE_CLKGEN_PSTATE 0 +#define APPLE_CLKGEN_PSTATE_DESIRED GENMASK(3, 0) + +#define SYS_DEV_PSTATE_SUSPEND 1 + +enum sys_device { + DEV_FABRIC, + DEV_DCS, + DEV_MAX, +}; + +struct apple_pmgr_sys_device { + void __iomem *base; + u32 active_state; + u32 suspend_state; +}; + +struct apple_pmgr_misc_hw { + u32 dev_min_ps[DEV_MAX]; +}; + +struct apple_pmgr_misc { + struct device *dev; + struct apple_pmgr_sys_device devices[DEV_MAX]; +}; + +static void apple_pmgr_sys_dev_set_pstate(struct apple_pmgr_misc *misc, + enum sys_device dev, bool active) +{ + u32 pstate; + u32 val; + + if (!misc->devices[dev].base) + return; + + if (active) + pstate = misc->devices[dev].active_state; + else + pstate = misc->devices[dev].suspend_state; + + dev_dbg(misc->dev, "set %d ps to pstate %d\n", dev, pstate); + + val = readl_relaxed(misc->devices[dev].base + APPLE_CLKGEN_PSTATE); + val &= ~APPLE_CLKGEN_PSTATE_DESIRED; + val |= FIELD_PREP(APPLE_CLKGEN_PSTATE_DESIRED, pstate); + writel_relaxed(val, misc->devices[dev].base); +} + +static int __maybe_unused apple_pmgr_misc_suspend_noirq(struct device *dev) +{ + struct apple_pmgr_misc *misc = dev_get_drvdata(dev); + int i; + + for (i = 0; i < DEV_MAX; i++) + apple_pmgr_sys_dev_set_pstate(misc, i, false); + + return 0; +} + +static int __maybe_unused apple_pmgr_misc_resume_noirq(struct device *dev) +{ + struct apple_pmgr_misc *misc = dev_get_drvdata(dev); + int i; + + for (i = 0; i < DEV_MAX; i++) + apple_pmgr_sys_dev_set_pstate(misc, i, true); + + return 0; +} + +static bool apple_pmgr_init_device(struct apple_pmgr_misc *misc, + const struct apple_pmgr_misc_hw *hw, + enum sys_device dev, + const char *device_name) +{ + void __iomem *base; + char name[32]; + u32 val; + + snprintf(name, sizeof(name), "%s-ps", device_name); + + base = devm_platform_ioremap_resource_byname( + to_platform_device(misc->dev), name); + if (!base) + return false; + + val = readl_relaxed(base + APPLE_CLKGEN_PSTATE); + + misc->devices[dev].base = base; + misc->devices[dev].active_state = + FIELD_GET(APPLE_CLKGEN_PSTATE_DESIRED, val); + misc->devices[dev].suspend_state = hw->dev_min_ps[dev]; + + return true; +} + +static int apple_pmgr_misc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + const struct apple_pmgr_misc_hw *hw; + struct apple_pmgr_misc *misc; + int ret = -ENODEV; + + misc = devm_kzalloc(dev, sizeof(*misc), GFP_KERNEL); + if (!misc) + return -ENOMEM; + + misc->dev = dev; + hw = of_device_get_match_data(dev); + + if (apple_pmgr_init_device(misc, hw, DEV_FABRIC, "fabric")) + ret = 0; + + if (apple_pmgr_init_device(misc, hw, DEV_DCS, "dcs")) + ret = 0; + + platform_set_drvdata(pdev, misc); + + return ret; +} + +static const struct apple_pmgr_misc_hw apple_pmgr_misc_hw_t600x = { + .dev_min_ps = { + [DEV_FABRIC] = SYS_DEV_PSTATE_SUSPEND, + [DEV_DCS] = 7, + }, +}; + +static const struct apple_pmgr_misc_hw apple_pmgr_misc_hw_t602x = { + .dev_min_ps = { + [DEV_FABRIC] = SYS_DEV_PSTATE_SUSPEND, + [DEV_DCS] = SYS_DEV_PSTATE_SUSPEND, + }, +}; + +static const struct of_device_id apple_pmgr_misc_of_match[] = { + { .compatible = "apple,t6000-pmgr-misc", .data = &apple_pmgr_misc_hw_t600x }, + { .compatible = "apple,t6020-pmgr-misc", .data = &apple_pmgr_misc_hw_t602x }, + {} +}; + +MODULE_DEVICE_TABLE(of, apple_pmgr_misc_of_match); + +static const struct dev_pm_ops apple_pmgr_misc_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(apple_pmgr_misc_suspend_noirq, + apple_pmgr_misc_resume_noirq) +}; + +static struct platform_driver apple_pmgr_misc_driver = { + .probe = apple_pmgr_misc_probe, + .driver = { + .name = "apple-pmgr-misc", + .of_match_table = apple_pmgr_misc_of_match, + .pm = pm_ptr(&apple_pmgr_misc_pm_ops), + }, +}; + +MODULE_AUTHOR("Hector Martin <marcan@marcan.st>"); +MODULE_DESCRIPTION("PMGR misc driver for Apple SoCs"); +MODULE_LICENSE("GPL"); + +module_platform_driver(apple_pmgr_misc_driver); -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] soc: apple: Add driver for Apple PMGR misc controls 2026-07-02 12:06 ` [PATCH 2/3] soc: apple: Add driver for Apple " Sasha Finkelstein @ 2026-07-02 12:23 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-07-02 12:23 UTC (permalink / raw) To: Sasha Finkelstein; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Incorrect check for devm_platform_ioremap_resource_byname() return value leads to ERR_PTR dereference - [High] Missing NULL check for of_device_get_match_data() causes NULL pointer dereference -- commit 144dd044ce23e2ad7967536cb164bb8942aedf81 Author: Hector Martin <marcan@marcan.st> soc: apple: Add driver for Apple PMGR misc controls This commit adds a driver for managing miscellaneous PMGR controls on Apple SoCs, implementing fabric and memory controller power state switching on system standby/s2idle to save power. > diff --git a/drivers/soc/apple/apple-pmgr-misc.c b/drivers/soc/apple/apple-pmgr-misc.c > new file mode 100644 > index 0000000000000..d487a87ecddb0 > --- /dev/null > +++ b/drivers/soc/apple/apple-pmgr-misc.c [ ... ] > +static bool apple_pmgr_init_device(struct apple_pmgr_misc *misc, > + const struct apple_pmgr_misc_hw *hw, > + enum sys_device dev, > + const char *device_name) > +{ > + void __iomem *base; > + char name[32]; > + u32 val; > + > + snprintf(name, sizeof(name), "%s-ps", device_name); > + > + base = devm_platform_ioremap_resource_byname( > + to_platform_device(misc->dev), name); > + if (!base) [Severity: High] Does this code handle the return value of devm_platform_ioremap_resource_byname() correctly? The function returns an error pointer on failure, not NULL. A failure here would bypass this check and could lead to an ERR_PTR being dereferenced by the readl_relaxed() call below. > + return false; > + > + val = readl_relaxed(base + APPLE_CLKGEN_PSTATE); > + > + misc->devices[dev].base = base; > + misc->devices[dev].active_state = > + FIELD_GET(APPLE_CLKGEN_PSTATE_DESIRED, val); > + misc->devices[dev].suspend_state = hw->dev_min_ps[dev]; [Severity: High] Can hw be NULL here? If the driver is bound manually via sysfs without a matching device tree node, of_device_get_match_data() in apple_pmgr_misc_probe() below returns NULL. This would lead to a NULL pointer dereference. > + > + return true; > +} > + > +static int apple_pmgr_misc_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + const struct apple_pmgr_misc_hw *hw; > + struct apple_pmgr_misc *misc; > + int ret = -ENODEV; > + > + misc = devm_kzalloc(dev, sizeof(*misc), GFP_KERNEL); > + if (!misc) > + return -ENOMEM; > + > + misc->dev = dev; > + hw = of_device_get_match_data(dev); > + > + if (apple_pmgr_init_device(misc, hw, DEV_FABRIC, "fabric")) [Severity: High] Is it possible for hw to be NULL here? Passing an unchecked hw pointer to apple_pmgr_init_device() causes the unconditional dereference of hw->dev_min_ps[dev] shown above. > + ret = 0; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702-pmgr-misc-v1-0-4f075a3a95c1@chaosmail.tech?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx 2026-07-02 12:06 [PATCH 0/3] soc: apple: Add "PMGR misc" power controls driver Sasha Finkelstein 2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein 2026-07-02 12:06 ` [PATCH 2/3] soc: apple: Add driver for Apple " Sasha Finkelstein @ 2026-07-02 12:06 ` Sasha Finkelstein 2026-07-02 12:29 ` sashiko-bot 2 siblings, 1 reply; 9+ messages in thread From: Sasha Finkelstein @ 2026-07-02 12:06 UTC (permalink / raw) To: Sven Peter, Janne Grunau, Neal Gompa, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, Sasha Finkelstein Adds the PMGR misc control nodes for M1/2 Pro/Max/Ultra series devices. Signed-off-by: Sasha Finkelstein <k@chaosmail.tech> --- arch/arm64/boot/dts/apple/t600x-die0.dtsi | 7 +++++++ arch/arm64/boot/dts/apple/t602x-die0.dtsi | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi index f715b19efd16..3271d234b483 100644 --- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi +++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi @@ -24,6 +24,13 @@ aic: interrupt-controller@28e100000 { power-domains = <&ps_aic>; }; + pmgr_misc: power-management@28e20c000 { + compatible = "apple,t6000-pmgr-misc"; + reg = <0x2 0x8e20c000 0 0x400>, + <0x2 0x8e20c800 0 0x400>; + reg-names = "fabric-ps", "dcs-ps"; + }; + smc: smc@290400000 { compatible = "apple,t6000-smc", "apple,smc"; reg = <0x2 0x90400000 0x0 0x4000>, diff --git a/arch/arm64/boot/dts/apple/t602x-die0.dtsi b/arch/arm64/boot/dts/apple/t602x-die0.dtsi index 8622ddea7b44..121f2cf75915 100644 --- a/arch/arm64/boot/dts/apple/t602x-die0.dtsi +++ b/arch/arm64/boot/dts/apple/t602x-die0.dtsi @@ -23,6 +23,13 @@ aic: interrupt-controller@28e100000 { power-domains = <&ps_aic>; }; + pmgr_misc: power-management@28e20c000 { + compatible = "apple,t6020-pmgr-misc"; + reg = <0x2 0x8e20c000 0 0x400>, + <0x2 0x8e20c400 0 0x400>; + reg-names = "fabric-ps", "dcs-ps"; + }; + nub_spmi0: spmi@29e114000 { compatible = "apple,t6020-spmi", "apple,t8103-spmi"; reg = <0x2 0x9e114000 0x0 0x100>; -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx 2026-07-02 12:06 ` [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx Sasha Finkelstein @ 2026-07-02 12:29 ` sashiko-bot 0 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-07-02 12:29 UTC (permalink / raw) To: Sasha Finkelstein; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Pre-existing error handling bug where the return value of `devm_platform_ioremap_resource_byname()` is incorrectly checked against NULL instead of IS_ERR, leading to an invalid pointer dereference. -- commit 7d3a9b5b08e4210f6d81e35e862b055c17a322ad Author: Sasha Finkelstein <k@chaosmail.tech> arm64: dts: apple: Add pmgr-misc nodes to t60xx This patch adds the PMGR misc control nodes to the device tree for the M1/2 Pro, Max, and Ultra series devices. It defines the register regions and power domains for these power-management nodes. > diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi > index f715b19efd167..3271d234b483a 100644 > --- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi > +++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi > @@ -24,6 +24,13 @@ aic: interrupt-controller@28e100000 { > power-domains = <&ps_aic>; > }; > > + pmgr_misc: power-management@28e20c000 { > + compatible = "apple,t6000-pmgr-misc"; > + reg = <0x2 0x8e20c000 0 0x400>, > + <0x2 0x8e20c800 0 0x400>; > + reg-names = "fabric-ps", "dcs-ps"; > + }; > + [Severity: High] This isn't a bug introduced by this patch, but adding this device tree node will trigger the probe function in drivers/soc/apple/apple-pmgr-misc.c, which has a pre-existing error handling issue. Could this cause a kernel panic during boot if the resource fails to map? In drivers/soc/apple/apple-pmgr-misc.c:apple_pmgr_init_device(), the resource mapping is handled like this: base = devm_platform_ioremap_resource_byname( to_platform_device(misc->dev), name); if (!base) return false; val = readl_relaxed(base + APPLE_CLKGEN_PSTATE); Should this check be IS_ERR(base) instead of !base? If the resource mapping fails, devm_platform_ioremap_resource_byname() returns an ERR_PTR (such as ERR_PTR(-EINVAL)). The !base check will evaluate to false, allowing the error pointer to be directly dereferenced as a memory address by readl_relaxed(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702-pmgr-misc-v1-0-4f075a3a95c1@chaosmail.tech?part=3 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-03 9:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-02 12:06 [PATCH 0/3] soc: apple: Add "PMGR misc" power controls driver Sasha Finkelstein 2026-07-02 12:06 ` [PATCH 1/3] dt-bindings: soc: apple: Add bindings for apple PMGR misc controls Sasha Finkelstein 2026-07-02 19:18 ` Conor Dooley 2026-07-02 19:35 ` Sasha Finkelstein 2026-07-03 9:52 ` Krzysztof Kozlowski 2026-07-02 12:06 ` [PATCH 2/3] soc: apple: Add driver for Apple " Sasha Finkelstein 2026-07-02 12:23 ` sashiko-bot 2026-07-02 12:06 ` [PATCH 3/3] arm64: dts: apple: Add pmgr-misc nodes to t60xx Sasha Finkelstein 2026-07-02 12:29 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox