codec: phytium: update phytium codec driver support to 6.6.0.4#1668
Open
Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Open
codec: phytium: update phytium codec driver support to 6.6.0.4#1668Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Chengyulai wants to merge 5 commits intodeepin-community:linux-6.6.yfrom
Conversation
Recording immediately after playing music will result in the latter part of the recording have no sound. the reason as follows: Playback and recording share the same clock. After playback stops, the audio framework will shutdown clock after 5-second. if recording within this 5-second period, the recording clock will be turned off. Therefore, the CONTROL1 register should be modified so that playback and recording use different clocks. Signed-off-by: Cheng Yulai <chengyulai1490@phytium.com.cn> Signed-off-by: Zhou Zheng <zhouzheng2069@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 avoids compiling this driver on other platforms. Signed-off-by: Cheng Yulai <chengyulai1490@phytium.com.cn> Signed-off-by: Zhou Zheng <zhouzheng2069@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Add a controller status code for no initialzation error. Signed-off-by: Cheng Yulai <chengyulai1490@phytium.com.cn> Signed-off-by: Zhou Zheng <zhouzheng2069@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Initialize channels and share memory before sending command to prevent unknown errors. Signed-off-by: Cheng Yulai <chengyulai1490@phytium.com.cn> Signed-off-by: Zhou Zheng <zhouzheng2069@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
When executing the probe interface, the driver shound return actual error code instead of zero to avoid creating sound card successfully when hardware is not present. Signed-off-by: Dai Jingtao <daijingtao1503@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Reviewer's GuideUpdates the Phytium codec and ES8388 drivers to use structured status codes, properly initialize shared memory and channel counts, and handle probe/control errors, along with a minor version bump and Kconfig updates. Sequence diagram for phytium codec probe and command initializationsequenceDiagram
participant Kernel
participant snd_soc_component as snd_soc_component
participant phytium_codec as phytium_codec
participant phytcodec_cmd as phytcodec_cmd
Kernel->>snd_soc_component: phyt_probe()
snd_soc_component->>phytium_codec: snd_soc_component_get_drvdata(component)
activate phytium_codec
phytium_codec->>phytcodec_cmd: msg = sharemem_base
activate phytcodec_cmd
phytium_codec->>phytcodec_cmd: memset(msg, 0, sizeof(struct_phytcodec_cmd))
deactivate phytcodec_cmd
phytium_codec->>phytium_codec: phyt_set_cmd(priv, PHYTCODEC_MSG_CMD_SET_PROBE)
deactivate phytium_codec
Sequence diagram for phytium codec startup with shared memory resetsequenceDiagram
participant App
participant ALSA
participant snd_soc_dai as snd_soc_dai
participant phytium_codec as phytium_codec
participant phytcodec_cmd as phytcodec_cmd
App->>ALSA: open_pcm_substream()
ALSA->>snd_soc_dai: phyt_startup(substream, dai)
snd_soc_dai->>phytium_codec: snd_soc_component_get_drvdata(component)
activate phytium_codec
phytium_codec->>phytcodec_cmd: msg = sharemem_base
activate phytcodec_cmd
phytium_codec->>phytcodec_cmd: memset(msg, 0, sizeof(struct_phytcodec_cmd))
deactivate phytcodec_cmd
alt playback_stream
phytium_codec->>phytium_codec: phyt_set_cmd(priv, PHYTCODEC_MSG_CMD_SET_STARTUP)
else capture_stream
phytium_codec->>phytium_codec: phyt_set_cmd(priv, PHYTCODEC_MSG_CMD_SET_STARTUP_REC)
end
deactivate phytium_codec
Class diagram for updated phytium codec status and initializationclassDiagram
class phytium_codec {
void~void*~ sharemem_base
unsigned_int channels
void~void*~ regfile_base
void~struct_device*~ dev
int phyt_set_cmd(priv, cmd)
int phyt_pm_cmd(priv, cmd)
}
class phytcodec_cmd {
uint8_t addr
uint8_t reg
uint8_t value
uint8_t reserved
}
class phytcodec_status {
<<enum>>
ERR_CODEC_SUCCESS
ERR_CODEC_BUS_BUSY
ERR_CODEC_DEV_BUSY
ERR_CODEC_RW_ERROR
ERR_CODEC_NODEV
ERR_CODEC_NO_INIT
}
class snd_soc_component {
phytium_codec* drvdata
}
class snd_soc_dai_driver {
snd_soc_pcm_stream playback
snd_soc_pcm_stream capture
}
class snd_soc_pcm_stream {
unsigned_int channels_max
}
phytium_codec --> phytcodec_cmd : sharemem_base
phytium_codec --> phytcodec_status : uses
snd_soc_component --> phytium_codec : get_drvdata
snd_soc_dai_driver --> snd_soc_pcm_stream : has
phytium_codec --> snd_soc_dai_driver : configures_channels
Flow diagram for es8388 component probe with error handlingflowchart TD
A["es8388_component_probe"] --> B["write ES8388_ADCPOWER 0xf0"]
B --> C{ret != 0?}
C -- yes --> D["return ret"]
C -- no --> E["write ES8388_CONTROL1 0x20"]
E --> F["write ES8388_DACCONTROL21 0x80"]
F --> G["write ES8388_ADCCONTROL10 0xda"]
G --> H["return 0"]
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. 😃 |
|
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:
- The repeated
memset(priv->sharemem_base, 0, sizeof(struct phytcodec_cmd))patterns across probe/remove/PM/DAI/debug paths could be factored into a small helper (e.g.phyt_clear_cmd(priv)) to avoid duplication and keep any future changes to shared memory initialization centralized. ERR_CODEC_BUS_BUSYis added toenum phytcodec_statusbut not used inphyt_codec_show_status()or elsewhere; either handle this status explicitly or drop it to avoid confusion about unimplemented error reporting.- In
es8388_component_probe()you now check the return value of the firstsnd_soc_component_write()but not the subsequent writes; consider handling or at least explicitly ignoring errors for the later writes to keep error handling consistent and intentional.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The repeated `memset(priv->sharemem_base, 0, sizeof(struct phytcodec_cmd))` patterns across probe/remove/PM/DAI/debug paths could be factored into a small helper (e.g. `phyt_clear_cmd(priv)`) to avoid duplication and keep any future changes to shared memory initialization centralized.
- `ERR_CODEC_BUS_BUSY` is added to `enum phytcodec_status` but not used in `phyt_codec_show_status()` or elsewhere; either handle this status explicitly or drop it to avoid confusion about unimplemented error reporting.
- In `es8388_component_probe()` you now check the return value of the first `snd_soc_component_write()` but not the subsequent writes; consider handling or at least explicitly ignoring errors for the later writes to keep error handling consistent and intentional.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 Codec V2 and ES8388 ASoC codec drivers to improve controller status handling, shared-memory initialization behavior, and probe robustness, along with a Kconfig dependency change.
Changes:
- Add controller/codec status codes and improve status logging paths in the Phytium Codec V2 driver.
- Clear the shared-memory command structure at additional lifecycle/DAI entry points; cache and reuse detected channel count.
- Update ES8388 driver version/default init values and begin checking probe-time register write failures.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| sound/soc/codecs/phytium-codec-v2.h | Adds new codec/controller status enum. |
| sound/soc/codecs/phytium-codec-v2.c | Adds shared-memory clearing at more entry points; updates status reporting; caches channel count. |
| sound/soc/codecs/Kconfig | Adds an architecture dependency for the Phytium Codec V2 driver. |
| sound/soc/codecs/es8388.c | Bumps version and adds partial error handling for probe-time register writes. |
Comments suppressed due to low confidence (1)
sound/soc/codecs/phytium-codec-v2.c:121
ERR_CODEC_BUS_BUSYwas added to the status enum but isn’t handled here, so a bus-busy condition will currently be logged as an "unknown error". Add an explicit switch case for this status (and a clear message) so status reporting remains accurate.
static void phyt_codec_show_status(uint8_t status)
{
switch (status) {
case ERR_CODEC_SUCCESS:
pr_err("success\n");
break;
case ERR_CODEC_DEV_BUSY:
pr_err("device busy\n");
break;
case ERR_CODEC_RW_ERROR:
pr_err("read/write error\n");
break;
case ERR_CODEC_NODEV:
pr_err("no hw device\n");
break;
case ERR_CODEC_NO_INIT:
pr_err("no init\n");
break;
default:
pr_err("unknown error: %d\n", status);
break;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+104
| ERR_CODEC_SUCCESS = 0, | ||
| ERR_CODEC_BUS_BUSY, | ||
| ERR_CODEC_DEV_BUSY, | ||
| ERR_CODEC_RW_ERROR, | ||
| ERR_CODEC_NODEV, //HW | ||
| ERR_CODEC_NO_INIT, //SW |
Comment on lines
328
to
334
| static int phyt_probe(struct snd_soc_component *component) | ||
| { | ||
| struct phytium_codec *priv = snd_soc_component_get_drvdata(component); | ||
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); | ||
|
|
Comment on lines
338
to
344
| static void phyt_remove(struct snd_soc_component *component) | ||
| { | ||
| struct phytium_codec *priv = snd_soc_component_get_drvdata(component); | ||
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); | ||
|
|
Comment on lines
348
to
354
| static int phyt_suspend(struct snd_soc_component *component) | ||
| { | ||
| struct phytium_codec *priv = snd_soc_component_get_drvdata(component); | ||
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); | ||
|
|
Comment on lines
358
to
364
| static int phyt_resume(struct snd_soc_component *component) | ||
| { | ||
| struct phytium_codec *priv = snd_soc_component_get_drvdata(component); | ||
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); | ||
|
|
Comment on lines
455
to
464
| static void phyt_shutdown(struct snd_pcm_substream *substream, | ||
| struct snd_soc_dai *dai) | ||
| { | ||
| int ret; | ||
| struct snd_soc_component *component = dai->component; | ||
| struct phytium_codec *priv = snd_soc_component_get_drvdata(component); | ||
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); | ||
|
|
Comment on lines
+516
to
+518
| struct phytcodec_cmd *msg = priv->sharemem_base; | ||
|
|
||
| memset(msg, 0, sizeof(struct phytcodec_cmd)); |
| dev_err(dev, "dump command requires one argument\n"); | ||
| goto error; | ||
| } | ||
| memset(priv->sharemem_base, 0, sizeof(struct phytcodec_cmd)); |
|
|
||
| config SND_SOC_PHYTIUM_CODEC_V2 | ||
| tristate "Phytium Codec V2 driver" | ||
| depends on ARCH_PHYTIUM |
Comment on lines
721
to
732
| static int es8388_component_probe(struct snd_soc_component *component) | ||
| { | ||
| snd_soc_component_write(component, ES8388_ADCPOWER, 0xf0); | ||
| snd_soc_component_write(component, ES8388_CONTROL1, 0x30); | ||
| int ret = 0; | ||
|
|
||
| ret = snd_soc_component_write(component, ES8388_ADCPOWER, 0xf0); | ||
| if (ret) | ||
| return ret; | ||
|
|
||
| snd_soc_component_write(component, ES8388_CONTROL1, 0x20); | ||
| snd_soc_component_write(component, ES8388_DACCONTROL21, 0x80); | ||
| snd_soc_component_write(component, ES8388_ADCCONTROL10, 0xda); | ||
|
|
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 codec driver.
1.Use different clocks for playback and recording
2.Add Kconfig option dependency
3.Add controller status code
4.initialize share memory and channels
5.Fix probe bug without judgment of return value
Summary by Sourcery
Update Phytium and ES8388 codec drivers for improved status handling, initialization, and robustness.
New Features:
Bug Fixes:
Enhancements: