i2s: phytium: update phytium i2s controller driver support to 6.6.0.4#1675
Open
Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Open
i2s: phytium: update phytium i2s controller driver support to 6.6.0.4#1675Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Conversation
Add audio control node to disable/enable I2S and DMA function. The node is used for dp-i2s to control audio whether it should stop or continue. Such as changing resolution when playing. Signed-off-by: Li Bing <libing1969@phytium.com.cn> Signed-off-by: Dai Jingtao <daijingtao1503@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Pulseaudio will read pcm device id and find the corresponding JACK based on this id. It determines whether to generate a pulseaudio device. i.e. whether the corresponding audio device appears in the system based on the plug status of the JACK. Add use_dai_pcm_id attribute to specify device id, so that even when devices are loaded out of order, the correct id can still be obtained for the device. This ensures correct pulseaudio behavior. Signed-off-by: Li Bing <libing1969@phytium.com.cn> Signed-off-by: Dai Jingtao <daijingtao1503@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
This driver is exclusively for the PHYTIUM platform and is not compatible with other SoCs. This restriction prevents errors on unsupported platform. Signed-off-by: Li Bing <libing1969@phytium.com.cn> Signed-off-by: Dai Jingtao <daijingtao1503@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
The problem is that when the hardware card is not inserted, it causes sound card loading to fail due to undefined behavior from headphone detection. This detection is in the I2S driver's probe function, but I2S cannot detect whether a daughter card actually exists. Therefore, the codec's probe should execute first and return directly if not daughter card is found. Signed-off-by: Li Bing <libing1969@phytium.com.cn> Signed-off-by: Dai Jingtao <daijingtao1503@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Reviewer's GuideUpdates the Phytium I2S v2 controller and related machine drivers to use DAI IDs for PCM mapping, adds a runtime control sysfs attribute to enable/disable I2S+DMA, adjusts jack init error handling, and sets the component probe order to defer I2S probing for better platform integration. Sequence diagram for sysfs control enabling/disabling I2S and DMAsequenceDiagram
actor User
participant Sysfs as Sysfs_phyt_i2s_control
participant Device as phytium_i2s_device
participant Driver as phytium_i2s_driver
participant DMA as DMA_controller
participant I2S as I2S_controller
User->>Sysfs: echo "1" > phyt_i2s_control
Sysfs->>Driver: phyt_i2s_control_store(buf="1", size)
Driver->>Device: dev_get_drvdata
Driver->>Driver: parse buf to value=1
alt enable_path
Driver->>DMA: phyt_writel_reg(dma_reg_base, PHYTIUM_DMA_CTL, 1)
Driver->>I2S: phyt_writel_reg(regfile_base, PHYTIUM_REGFILE_ITER, TX_EN)
end
Driver-->>Sysfs: return size
Sysfs-->>User: write completed
User->>Sysfs: echo "0" > phyt_i2s_control
Sysfs->>Driver: phyt_i2s_control_store(buf="0", size)
Driver->>Device: dev_get_drvdata
Driver->>Driver: parse buf to value=0
alt disable_path
Driver->>I2S: phyt_writel_reg(regfile_base, PHYTIUM_REGFILE_ITER, TX_DIS)
Driver->>DMA: phyt_writel_reg(dma_reg_base, PHYTIUM_DMA_CTL, 0)
end
Driver-->>Sysfs: return size
Sysfs-->>User: write completed
Updated class diagram for Phytium I2S v2 component and DAI linksclassDiagram
class phytium_i2s_component {
+string name
+bool use_dai_pcm_id
+int probe_order
+function pcm_construct(component)
+function pcm_destruct(component)
+function suspend(component)
+function resume(component)
}
class phytium_i2s_driver {
+string version
+struct phytium_i2s *priv
+function phyt_i2s_control_store(dev, attr, buf, size)
+function phyt_i2s_debug_store(dev, attr, buf, size)
}
class phytium_i2s {
+void *dma_reg_base
+void *regfile_base
}
class snd_soc_dai_link_pmdk_dai0 {
+string name
+int id
+string stream_name
+int dai_fmt
+function init(runtime)
}
class snd_soc_dai_link_pmdk_dai1 {
+string name
+int id
+string stream_name
+int dai_fmt
+function init(runtime)
}
class snd_soc_dai_link_pmdk_dai2 {
+string name
+int id
+string stream_name
+int dai_fmt
+function init(runtime)
}
class snd_soc_dai_link_phyt_machine_dai0 {
+string name
+int id
+string stream_name
+int dai_fmt
}
class phytium_sysfs_attributes {
+device_attribute phyt_i2s_debug
+device_attribute phyt_i2s_control
}
phytium_i2s_driver --> phytium_i2s : uses
phytium_i2s_driver --> phytium_sysfs_attributes : exposes
phytium_i2s_component --> phytium_i2s_driver : registered_by
snd_soc_dai_link_pmdk_dai0 --> phytium_i2s_component : uses_dai_id_0
snd_soc_dai_link_pmdk_dai1 --> phytium_i2s_component : uses_dai_id_1
snd_soc_dai_link_pmdk_dai2 --> phytium_i2s_component : uses_dai_id_2
snd_soc_dai_link_phyt_machine_dai0 --> phytium_i2s_component : uses_dai_id_0
note for phytium_i2s_component "use_dai_pcm_id = true, probe_order = SND_SOC_COMP_ORDER_LATE"
note for phytium_i2s_driver "PHYT_I2S_V2_VERSION updated to 1.0.9"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Chengyulai. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In phyt_i2s_control_store(), the use of
strscpy(p, buf, sizeof(p))is incorrect sincesizeof(p)is the size of the pointer, not the allocated buffer; this should be replaced with the actual buffer size (e.g.size) or the temporary buffer eliminated entirely in favor of parsing directly frombuf. - The new sysfs control path accepts any value other than 0/1 without error and still reports success; consider validating the parsed value and returning
-EINVALfor unsupported settings to avoid silent no-op writes. - The
.origfilephytium-i2s-v2.c.origappears to be accidentally added in the diff and should be removed from the tree.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In phyt_i2s_control_store(), the use of `strscpy(p, buf, sizeof(p))` is incorrect since `sizeof(p)` is the size of the pointer, not the allocated buffer; this should be replaced with the actual buffer size (e.g. `size`) or the temporary buffer eliminated entirely in favor of parsing directly from `buf`.
- The new sysfs control path accepts any value other than 0/1 without error and still reports success; consider validating the parsed value and returning `-EINVAL` for unsupported settings to avoid silent no-op writes.
- The `.orig` file `phytium-i2s-v2.c.orig` appears to be accidentally added in the diff and should be removed from the tree.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Updates the Phytium I2S v2 ASoC stack to align with the 6.6.0.4 audio stack by adding PCM-ID-based routing support, deferring component probing, and improving jack init error propagation, plus introducing a sysfs control hook.
Changes:
- Propagate
snd_soc_component_set_jack()return values in DP init paths and add explicit DAI link IDs. - Enable
.use_dai_pcm_idand defer the I2S v2 component probe to late order; bump driver version to 1.0.9. - Add a sysfs control node and restrict the v2 driver Kconfig to
ARCH_PHYTIUM(but also adds an unused.origsource file).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sound/soc/phytium/pmdk_dp.c | Return jack setup errors and assign DAI link IDs for DP links. |
| sound/soc/phytium/phytium-machine-v2.c | Assign a DAI link ID for the v2 machine link. |
| sound/soc/phytium/phytium-i2s-v2.c | Version bump; enable use_dai_pcm_id; defer probe; add sysfs control attribute (contains a critical sysfs parsing/free bug). |
| sound/soc/phytium/phytium-i2s-v2.c.orig | New backup-like source file added (appears unused/unreferenced). |
| sound/soc/phytium/Kconfig | Restrict SND_SOC_PHYTIUM_I2S_V2 to ARCH_PHYTIUM. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+966
to
+970
| p = kmalloc(size, GFP_KERNEL); | ||
| if (p == NULL) | ||
| return -EINVAL; | ||
| strscpy(p, buf, sizeof(p)); | ||
|
|
Comment on lines
+971
to
+975
| token = strsep(&p, " "); | ||
| if (!token) { | ||
| ret = -EINVAL; | ||
| goto error; | ||
| } |
Comment on lines
+1
to
+5
| // SPDX-License-Identifier: GPL-2.0 | ||
| /* | ||
| * Phytium I2S ASoC driver | ||
| * | ||
| * Copyright (C) 2023-2024, Phytium Technology Co., Ltd. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patches updates the support for phytium i2s controller driver.
1.Add audio control node
2.Add use_dai_pcm_id attribute
3.Restricted adaptation platform
4.Defer I2S probe procedure
Summary by Sourcery
Update Phytium I2S v2 and related machine drivers to improve control, jack handling, and DAI linkage against the 6.6.0.4 audio stack.
New Features:
Bug Fixes:
Enhancements: