[26.04_linux-nvidia-bos] soc: mediatek: SSPM control interface and power_wrap driver - #571
[26.04_linux-nvidia-bos] soc: mediatek: SSPM control interface and power_wrap driver#571dcemin-nv wants to merge 9 commits into
Conversation
BaseOS Kernel ReviewWarning
|
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint ❌ Errors foundDetailsChecking 9 commits... Cherry-pick digest: ┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐ │ Local │ Referenced upstream / Patch subject │ Patch-ID │ Subject │ SoB chain │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ e26d26d4abc7 │ [UBUNTU] [Config] annotations for the MediaTek SSPM and power_wr │ N/A │ N/A │ dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 32ae73254206 │ [SAUCE] soc: mediatek: power_wrap: release usb4 resources via ss │ N/A │ N/A │ wani, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 2c42d3af1a54 │ [SAUCE] soc: mediatek: power_wrap: request ac_bestperf mode duri │ N/A │ N/A │ diwate, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 131348fef081 │ [SAUCE] soc: mediatek: power_wrap: enable dvfsrc via spm sysram │ N/A │ N/A │ wani, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 2a3b9e45e93c │ [SAUCE] soc: mediatek: power_wrap: expose client requests in sys │ N/A │ N/A │ wani, kbutala, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 2a889a08ecc2 │ [SAUCE] soc: mediatek: power_wrap: add client power control api │ N/A │ N/A │ wani, kbutala, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 10e007fa859f │ [SAUCE] soc: mediatek: power_wrap: add sysfs debug interface │ N/A │ N/A │ wani, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 15104129b4a8 │ [SAUCE] soc: mediatek: add power_wrap device power control drive │ N/A │ N/A │ wani, dcemin │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 0075a970d0ec │ [SAUCE] soc: mediatek: add sspm control interface │ N/A │ N/A │ wani, dcemin │ └──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘ Lint: all checks passed. PR metadata: E: PR targets 26.04_linux-nvidia-bos but body has no https://bugs.launchpad.net/... link |
| * in-flight SET transaction early (before firmware consumed the SET | ||
| * message), so the ack is gated to the SET id. | ||
| */ | ||
| if (id == SSPM_CI_SET_ID) |
There was a problem hiding this comment.
Codex independently found this issue as well; it matches the Boro review at the same head. _hw_ack is a transaction-independent flag. After request A times out, its delayed SET IRQ can set the flag while request B is waiting, so B returns success without receiving its own response. The generated SCMI token does not help unless the handler reads the response header and completes only the matching transaction.
| * device_type strings are heap-allocated by | ||
| * pwrap_acpi_fetch_dpm_config(); free them here. | ||
| */ | ||
| pwrap_acpi_dpm_config_free(pwrap_get_dev_ctrl()); |
There was a problem hiding this comment.
Codex independently found this issue as well; it matches the Boro teardown-lifetime finding. mtk_pwrap_dev_probe() returns raw pointers into dev_ctrl.configs, but remove frees that storage without a release operation, reference counting, or removal notification. A consumer can retain and later dereference the pointer after provider removal. suppress_bind_attrs only blocks manual sysfs unbind; it does not make the pointer lifetime safe.
| unsigned int id = *((unsigned int *)data); | ||
|
|
||
| writel(SSPM_CI_MAGIC, (void __iomem *)MBOX_OUT_CLR(id)); | ||
| writel(0x0, (void __iomem *)MBOX_BOX(id, 0x4)); |
There was a problem hiding this comment.
Codex independently found this issue as well; it matches the Boro review at the same head. The interrupt handler overwrites the response status at offset 0x4 with zero before the caller can inspect it, and sspm_ci_set() returns zero after any acknowledgement. Firmware-rejected requests are therefore reported as successful. Please preserve and return the firmware response status.
| message_header = pack_message_header(message_id, message_type, | ||
| protocol_id, token); | ||
| scmi_tx.message_header = message_header; | ||
| while (readl((void __iomem *)MBOX_IN_SET(SSPM_CI_SET_ID))) { |
There was a problem hiding this comment.
Codex independently found this issue as well; it matches the Boro review at the same head. Both timeout paths spin with mdelay(1) while holding _hw_rsc. Each can occupy the CPU for about 10 seconds, and the worst case across both loops is about 20 seconds, while serializing every other caller behind the mutex. Please use a sleeping/polling timeout or a completion-based wait.
| if (ret) { | ||
| dev_err(&pdev->dev, "failed to setup resource for type: 0x%x with error: 0x%x\n", | ||
| (enum mtk_pwrap_type)id->driver_data, ret); | ||
| return ret; |
There was a problem hiding this comment.
Codex independently found this issue as well; it matches the Boro review at the same head. acpi_get_name() with ACPI_ALLOCATE_BUFFER allocates priv->acpi_path.pointer, but this return bypasses ACPI_FREE(). Every probe failure from pwrap_config_resource() can therefore leak another pathname buffer. Please add cleanup on this error path.
Add the SSPM control interface: the SCMI mailbox transport used to talk to the MediaTek System Services Power Manager microcontroller. The transport carries SCMI messages through a shared SRAM message area with doorbell registers, one mailbox pair for the SET and GET channels, with interrupts requested by resource index from the ACPI device that describes the mailbox window. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
Add the power_wrap driver: ACPI-driven device power control through the SSPM control interface. Kernel clients and the ACPI power flows use it to request device power, clock and mode changes from the SSPM firmware, which reference counts the requests across operating system components. The public client header is introduced directly at include/linux/soc/mediatek/mtk-pwrap.h so in-tree consumers outside drivers/soc can include it without relative paths. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Co-developed-by: David Cemin <dcemin@nvidia.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
Add a sysfs interface to the power_wrap driver for inspecting and exercising device power control requests from userspace. This is a debug and validation aid; the functional path is the in-kernel client API and the ACPI flows. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
Add mtk_send_power_control_req(), the in-kernel client entry point for pushing a power control command to the SSPM. Kernel drivers that manage SSPM-controlled resources (for example the audio clock tree and the audio DSP) fill a request descriptor and submit it through this interface instead of open coding the SCMI exchange. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: Kaushal Rajeev Butala <kbutala@nvidia.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
…ysfs Extend the sysfs debug interface to cover the client power control request path, so validation can issue and observe client requests without a kernel consumer. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: Kaushal Rajeev Butala <kbutala@nvidia.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
…M flag Set the SPM SYSRAM flag that enables DVFSRC so dynamic voltage and frequency scaling resource control is active when the driver has initialized. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
…ring boot Request the AC_BESTPERF performance mode from the SSPM during boot so the platform boots in the best performance mode on AC power, matching the intended out of box behavior. Signed-off-by: sanket.diwate <sanket.diwate@mediatek.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
…SSPM Release the usb4 resources through the SSPM so the platform can enter its deepest sleep states when usb4 is idle. Signed-off-by: mayuresh.wani <mayuresh.wani@mediatek.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
…vers Add the config annotations for MTK_SSPM_CI and MTK_POWER_WRAP. Signed-off-by: David Cemin <dcemin@nvidia.com>
1b20d06 to
e26d26d
Compare
|
Authorship corrected per Kaushal: the client power control API and the sysfs exposure commits are Mayuresh Wani's work and now carry him as author with his Signed-off-by at the head of the chain; Kaushal and I remain as sign-offs for the carry. No content change; the branches were also refreshed onto the current base tips. |
2 participants
Standalone submission of the MediaTek SSPM control interface and the power_wrap device power control driver, previously carried as a prerequisite inside the audio series and split out per review feedback.
Series structure:
All commits carry author sign-offs (MediaTek and NVIDIA authors acked). Content is identical to the tree validated on the reference board (audio playback, deep sleep and resume exercised through the client API path).
The audio series PRs #566 and #567 depend on the client power control API introduced here and should merge after this set.