Restore RK3308 Ethernet and audio functionality#9413
Merged
brentr merged 2 commits intoarmbian:mainfrom Feb 18, 2026
Merged
Conversation
and 8002936 because: The ...fix-10mbit-ethernet patch is still necessary Without it, the RK3308 does not connect via DHCP to 10-mbit Ethernets The ...pinctl-slow-mux patch still compiles without error It is <100 bytes and helps debugging GPIO issues. I don't understand why above patches were flagged as "broken"
brentr
requested review from
ColorfulRhino,
HeyMeco,
JohnTheCoolingFan,
SuperKali,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
catalinii,
chainsx,
efectn,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal,
pyavitz,
rpardini and
schwar3kat
as code owners
github-actions
bot
added
size/medium
Contributor
📝 WalkthroughWalkthroughThis patch reverts the RK3308 Acodec driver to vendor code for kernel 6.19, updating the codec driver implementation, switching kernel control access patterns, refreshing register definitions and configurations, and introducing a new public provider header. Module licensing updated from "GPL" to "GPL v2". Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
Needs review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
patch/kernel/archive/rockchip64-6.19/rk3308-acodec-vendor-driver.patch (3)
5054-5075:⚠️ Potential issue | 🟡 MinorAdd bounds checks in sysfs adc_grps_store.
Line 5071/5074 accept arbitrary values and can pushused_adc_grpsorloopback_grpbeyond array bounds, risking OOB access in later loops.🔧 Proposed fix
if (adc_type == 'n') - rk3308->used_adc_grps = grps; - else if (adc_type == 'l') - rk3308->loopback_grp = grps; + { + if (grps < 1 || grps > ADC_LR_GROUP_MAX) + return -EINVAL; + rk3308->used_adc_grps = grps; + } + else if (adc_type == 'l') + { + if (grps != NOT_USED && + (grps < 0 || grps >= ADC_LR_GROUP_MAX)) + return -EINVAL; + rk3308->loopback_grp = grps; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.19/rk3308-acodec-vendor-driver.patch` around lines 5054 - 5075, The sysfs setter adc_grps_store parses adc_type and grps into rk3308->used_adc_grps or rk3308->loopback_grp without bounds checking, allowing OOB indices later; update adc_grps_store (in struct rk3308_codec_priv context) to validate that grps is within the valid range (e.g., 0 <= grps < MAX_ADC_GRPS or <= ARRAY_SIZE(whatever array) - 1) before assigning, return -EINVAL on invalid input and log a clear dev_err including the bad value and expected range; also handle non-recognized adc_type by returning -EINVAL.
3249-3262:⚠️ Potential issue | 🟠 MajorFix DAC ramp-down sequence (wrong register & range).
Line 3249 uses ADC register/mask and loops to 0x7f, but the comment and power-on path indicate DAC_ANA_CON14[3:0] should ramp 0x1→0xf. As written, DAC VCM won’t ramp down correctly for version B and ADC gets touched instead.🔧 Proposed fix
- for (v = 0x1; v <= 0x7f; v++) { - regmap_update_bits(rk3308->regmap, - RK3308_ADC_ANA_CON10(0), - RK3308_ADC_CURRENT_CHARGE_MSK, - v); + for (v = 0x1; v <= 0xf; v++) { + regmap_update_bits(rk3308->regmap, + RK3308_DAC_ANA_CON14, + RK3308_DAC_CURRENT_CHARGE_MSK, + v); udelay(200); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.19/rk3308-acodec-vendor-driver.patch` around lines 3249 - 3262, The ramp-down loop for rk3308 version B is writing the ADC register and using ADC mask; change the loop to target the DAC register and mask and constrain the range to 0x1→0xf: replace RK3308_ADC_ANA_CON10 and RK3308_ADC_CURRENT_CHARGE_MSK used in regmap_update_bits with the DAC equivalents (e.g. RK3308_DAC_ANA_CON14 and the DAC VCM mask constant) and iterate v from 0x1 to 0xf (keeping the udelay(200) per step) so the DAC VCM is ramped correctly for codec_ver == ACODEC_VERSION_B while leaving the surrounding logic unchanged.
5919-5928:⚠️ Potential issue | 🟠 MajorCancel delayed work + clear exported callback on remove.
Line 5919 removes the platform driver without cancellinghpdet_work/loopback_work. This can run after teardown and touch freed state. Also, the exportedrk3308_codec_set_jack_detect_cbshould be cleared to avoid a stale function pointer after module unload.🔧 Proposed fix
static void rk3308_platform_remove(struct platform_device *pdev) { struct rk3308_codec_priv *rk3308 = (struct rk3308_codec_priv *)platform_get_drvdata(pdev); + cancel_delayed_work_sync(&rk3308->hpdet_work); + cancel_delayed_work_sync(&rk3308->loopback_work); + rk3308_codec_set_jack_detect_cb = NULL; + clk_disable_unprepare(rk3308->mclk_rx); clk_disable_unprepare(rk3308->mclk_tx); clk_disable_unprepare(rk3308->pclk); device_unregister(&rk3308->dev);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.19/rk3308-acodec-vendor-driver.patch` around lines 5919 - 5928, In rk3308_platform_remove, stop and cancel the driver's delayed/queued works and clear the exported jack callback to avoid use-after-free/stale pointers: retrieve the private via platform_get_drvdata(rk3308) in rk3308_platform_remove, call cancel_delayed_work_sync(&rk3308->hpdet_work) and cancel_delayed_work_sync(&rk3308->loopback_work) (or cancel_work_sync if they are plain works), then set the exported callback rk3308_codec_set_jack_detect_cb to NULL before unregistering the device and disabling clocks (rk3308_platform_remove, rk3308->hpdet_work, rk3308->loopback_work, rk3308_codec_set_jack_detect_cb, clk_disable_unprepare, device_unregister).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@patch/kernel/archive/rockchip64-6.19/rk3308-acodec-vendor-driver.patch`:
- Around line 5054-5075: The sysfs setter adc_grps_store parses adc_type and
grps into rk3308->used_adc_grps or rk3308->loopback_grp without bounds checking,
allowing OOB indices later; update adc_grps_store (in struct rk3308_codec_priv
context) to validate that grps is within the valid range (e.g., 0 <= grps <
MAX_ADC_GRPS or <= ARRAY_SIZE(whatever array) - 1) before assigning, return
-EINVAL on invalid input and log a clear dev_err including the bad value and
expected range; also handle non-recognized adc_type by returning -EINVAL.
- Around line 3249-3262: The ramp-down loop for rk3308 version B is writing the
ADC register and using ADC mask; change the loop to target the DAC register and
mask and constrain the range to 0x1→0xf: replace RK3308_ADC_ANA_CON10 and
RK3308_ADC_CURRENT_CHARGE_MSK used in regmap_update_bits with the DAC
equivalents (e.g. RK3308_DAC_ANA_CON14 and the DAC VCM mask constant) and
iterate v from 0x1 to 0xf (keeping the udelay(200) per step) so the DAC VCM is
ramped correctly for codec_ver == ACODEC_VERSION_B while leaving the surrounding
logic unchanged.
- Around line 5919-5928: In rk3308_platform_remove, stop and cancel the driver's
delayed/queued works and clear the exported jack callback to avoid
use-after-free/stale pointers: retrieve the private via
platform_get_drvdata(rk3308) in rk3308_platform_remove, call
cancel_delayed_work_sync(&rk3308->hpdet_work) and
cancel_delayed_work_sync(&rk3308->loopback_work) (or cancel_work_sync if they
are plain works), then set the exported callback rk3308_codec_set_jack_detect_cb
to NULL before unregistering the device and disabling clocks
(rk3308_platform_remove, rk3308->hpdet_work, rk3308->loopback_work,
rk3308_codec_set_jack_detect_cb, clk_disable_unprepare, device_unregister).
EvilOlaf
approved these changes
Feb 18, 2026
Member
|
Great. Thanks for fixing those. |
Contributor
|
✅ This PR has been reviewed and approved — all set for merge! |
github-actions
bot
added
Ready to merge
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
2 participants
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.
Description
@EvilOlaf
Recent commits into rockchip64-6.19 have broken caused the following problems:
This patch reverts 32f3b7c
to restore proper 10Mbit Ethernet
It also reverts 8002936
as this feature adds <100 bytes to the kernel to aid in debug of GPIO drive issues
Getting the analog audio to compile under the 6.19 kernel required only minimal edits to the existing RK3308 vendor acodec patch. Many other acodec drivers were similarly effected.
As with the others, the removed snd_soc_kcontrol_component() was simply replaced with
snd_kcontrol_chip() This may not be the "best" solution, but there are many other drivers in the kernel that have taken this approach. It's still my hope that this vendor driver will eventually disappear. Unfortunately, the upstream driver STILL lacks support for the variant of the RK3308 used by all Radaxa boards.
GitHub issue reference:
Jira reference number [AR-9999]
How Has This Been Tested?
Built "edge" image with Debian trixie for a v1.3 Radxa RockPI-S
Verify 10Mbit Ethernet failed beforehand, but works with this patch.
Similarly verified missing audio beforehand, but works after this patch.
Checklist:
Please delete options that are not relevant.
Summary by CodeRabbit