sm8550-6.18: Fix TF card IO performance regression#9546
Open
kasimling wants to merge 1 commit intoarmbian:mainfrom
Open
sm8550-6.18: Fix TF card IO performance regression#9546kasimling wants to merge 1 commit intoarmbian:mainfrom
kasimling wants to merge 1 commit intoarmbian:mainfrom
Conversation
- Fix the issue that downstream MSM sdhc driver cannot work on 6.18.14 or newer kernel - Switch back to downstream MSM sdhc driver for Ayn Odin2 devices to fix the IO performance regression brought by upstream driver. Before this fix, average sequential read bandwidth on a UHS-I SDR104 card is 16.8MiB/s. After this fix applied, it is 85.1MiB/s. It fixes commit f785a6d Signed-off-by: Alex Ling <ling_kasim@hotmail.com>
Contributor
📝 WalkthroughWalkthroughThis changeset enables downstream SDCC (SD/MMC) driver support on the SM8550 kernel by updating kernel configuration, removing redundant cleanup calls from the SDHCI driver, reverting clock operation types, and restructuring device tree files to configure the downstream SDHC driver with comprehensive power management and interconnect settings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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
05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
patch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch (1)
126-206: Consider extracting shared SDHC configuration to reduce duplication.The
sdhc_2node is nearly identical betweenqcs8550-ayn-odin2.dtsandqcs8550-ayn-odin2portal.dts(~80 lines). If both Odin2 variants will continue to share this configuration, consider extracting it to a common dtsi file (e.g.,qcs8550-ayn-odin2-sdhc.dtsi) to improve maintainability.This is optional and can be deferred if the configurations are expected to diverge in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch` around lines 126 - 206, Extract the duplicated sdhc_2 node (sdhci@8804000) and its child opp-table (sdhc2_opp_table) into a new common dtsi (e.g., qcs8550-ayn-odin2-sdhc.dtsi), then include that dtsi from both qcs8550-ayn-odin2.dts and qcs8550-ayn-odin2portal.dts and remove the duplicate node blocks; ensure you preserve labels (sdhc_2, sdhci@8804000, sdhc2_opp_table), phandles (vdd-supply, vdd-io-supply, &gcc, &apps_smmu, etc.), pinctrl references (&sdc2_default, &sdc2_sleep, &sdc2_card_det_n) and interconnect/MSM-bus settings so all references resolve, and if future divergence is expected keep only truly shared properties in the new dtsi and leave variant-specific overrides in each DTS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@patch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch`:
- Around line 40-46: The comment above qcom,dll-hsr-list incorrectly includes an
extra "0" ("0 but it is calculated"); edit the comment in the patch's block that
mentions DLL_CONFIG_2 so it reads "...DLL_CONFIG_2 value is not passed from the
device tree, but it is calculated in the driver." (reference symbols:
qcom,dll-hsr-list and DLL_CONFIG_2) — remove the stray "0" and keep punctuation
consistent with qcs8550-ayn-odin2portal.dts.
---
Nitpick comments:
In
`@patch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch`:
- Around line 126-206: Extract the duplicated sdhc_2 node (sdhci@8804000) and
its child opp-table (sdhc2_opp_table) into a new common dtsi (e.g.,
qcs8550-ayn-odin2-sdhc.dtsi), then include that dtsi from both
qcs8550-ayn-odin2.dts and qcs8550-ayn-odin2portal.dts and remove the duplicate
node blocks; ensure you preserve labels (sdhc_2, sdhci@8804000,
sdhc2_opp_table), phandles (vdd-supply, vdd-io-supply, &gcc, &apps_smmu, etc.),
pinctrl references (&sdc2_default, &sdc2_sleep, &sdc2_card_det_n) and
interconnect/MSM-bus settings so all references resolve, and if future
divergence is expected keep only truly shared properties in the new dtsi and
leave variant-specific overrides in each DTS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19e910b5-5627-4f3c-9b46-90a851564624
📒 Files selected for processing (4)
config/kernel/linux-sm8550-edge.configpatch/kernel/archive/sm8550-6.18/0215-drivers-mmc-Added-qcom-downstream-sdhci-driver.patchpatch/kernel/archive/sm8550-6.18/0216-Revert-clk-qcom-gcc-sm8550-Use-floor-ops-for-SDCC-RC.patchpatch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch
Comment on lines
+40
to
+46
| + /* | ||
| + * DLL HSR settings. Refer go/hsr - <Target> DLL settings. | ||
| + * Note that the DLL_CONFIG_2 value is not passed from the | ||
| + * device tree, 0 but it is calculated in the driver. | ||
| + */ | ||
| + qcom,dll-hsr-list = <0x0007442C 0x0 0x10 | ||
| + 0x090106C0 0x80040868>; |
Contributor
There was a problem hiding this comment.
Minor typo in comment.
Line 43 has an extra "0" in the comment text: "0 but it is calculated" should read "but it is calculated" to match the equivalent comment in qcs8550-ayn-odin2portal.dts (line 146).
📝 Proposed fix
/*
* DLL HSR settings. Refer go/hsr - <Target> DLL settings.
* Note that the DLL_CONFIG_2 value is not passed from the
- * device tree, 0 but it is calculated in the driver.
+ * device tree, but it is calculated in the driver.
*/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@patch/kernel/archive/sm8550-6.18/0217-arm64-dts-Switch-to-downstream-sdhc-driver-for-Odin2.patch`
around lines 40 - 46, The comment above qcom,dll-hsr-list incorrectly includes
an extra "0" ("0 but it is calculated"); edit the comment in the patch's block
that mentions DLL_CONFIG_2 so it reads "...DLL_CONFIG_2 value is not passed from
the device tree, but it is calculated in the driver." (reference symbols:
qcom,dll-hsr-list and DLL_CONFIG_2) — remove the stray "0" and keep punctuation
consistent with qcs8550-ayn-odin2portal.dts.
igorpecovnik
approved these changes
Mar 16, 2026
Contributor
|
✅ This PR has been reviewed and approved — all set for 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.
Before this fix, average sequential read bandwidth on a UHS-I SDR104 card is 16.8MiB/s. After this fix applied, it is 85.1MiB/s.
It fixes commit f785a6d
Summary by CodeRabbit
Release Notes