Retroidpocket: fix current and add edge as target#9724
Open
EvilOlaf wants to merge 2 commits intoarmbian:mainfrom
Open
Retroidpocket: fix current and add edge as target#9724EvilOlaf wants to merge 2 commits intoarmbian:mainfrom
current and add edge as target#9724EvilOlaf wants to merge 2 commits intoarmbian:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBoard configs for two Retroid Pocket models now expand Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
size/large
![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 (2)
patch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-common.dtsi (2)
612-612: Nit: stray whitespace on a few lines.Cosmetic only — no impact on the compiled DT, but worth tidying:
- Line 612: leading space before
color = <LED_COLOR_ID_RED>;(\t\t\tcolor).- Line 753: space mixed into tab indent before
enable-gpios = ….- Line 1124: trailing tab after
"IN2_HPHR", "HPHR_OUT",.Also applies to: 753-753, 1124-1124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-common.dtsi` at line 612, Remove the stray whitespace found in the device tree fragment: delete the leading space before the color assignment in the node containing color = <LED_COLOR_ID_RED>; (ensure the line begins with the same tabs as surrounding properties), remove the extra space mixed into the tab indent on the line with enable-gpios = … so its indentation is consistent with other properties, and remove the trailing tab after the string list containing "IN2_HPHR", "HPHR_OUT", so there are no trailing whitespace characters; target the exact properties color = <LED_COLOR_ID_RED>, enable-gpios, and the string list "IN2_HPHR", "HPHR_OUT" when making these whitespace-only edits.
1044-1054: Minor: mixed tab/space indentation infan_pwm_active.Lines 1048-1052 indent the properties with spaces while the surrounding nodes (and the rest of this DTSI) use tabs. Harmless to the DTC but inconsistent with project style.
🎨 Normalize to tabs
fan_pwm_active: fan-pwm-active-state { pins = "gpio6"; function = "func1"; - bias-disable; - power-source = <0>; - output-low; - qcom,drive-strength = <3>; - drive-push-pull; + bias-disable; + power-source = <0>; + output-low; + qcom,drive-strength = <3>; + drive-push-pull; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-common.dtsi` around lines 1044 - 1054, The properties inside the fan_pwm_active node under &pm8150l_gpios use spaces for indentation while the rest of the DTSI uses tabs; update the indentation of the fan_pwm_active node (fan-pwm-active-state) properties—pins, function, bias-disable, power-source, output-low, qcom,drive-strength, drive-push-pull—to use tabs to match the surrounding file style and maintain consistency with the &pm8150l_gpios node formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/boards/retroidpocket-rp5.csc`:
- Line 7: KERNEL_TARGET is set as a space-separated string ("current edge")
which breaks the documented comma-separated parsing used by config-prepare.sh
(it expects IFS=',' to split into KERNEL_TARGET_ARRAY); update the KERNEL_TARGET
value to a comma-separated list (e.g., "current,edge") in this board config so
downstream code that reads KERNEL_TARGET_ARRAY and performs branch validation in
lib/functions/main/config-prepare.sh and config-interactive.sh works correctly
and matches the other board files.
---
Nitpick comments:
In `@patch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-common.dtsi`:
- Line 612: Remove the stray whitespace found in the device tree fragment:
delete the leading space before the color assignment in the node containing
color = <LED_COLOR_ID_RED>; (ensure the line begins with the same tabs as
surrounding properties), remove the extra space mixed into the tab indent on the
line with enable-gpios = … so its indentation is consistent with other
properties, and remove the trailing tab after the string list containing
"IN2_HPHR", "HPHR_OUT", so there are no trailing whitespace characters; target
the exact properties color = <LED_COLOR_ID_RED>, enable-gpios, and the string
list "IN2_HPHR", "HPHR_OUT" when making these whitespace-only edits.
- Around line 1044-1054: The properties inside the fan_pwm_active node under
&pm8150l_gpios use spaces for indentation while the rest of the DTSI uses tabs;
update the indentation of the fan_pwm_active node (fan-pwm-active-state)
properties—pins, function, bias-disable, power-source, output-low,
qcom,drive-strength, drive-push-pull—to use tabs to match the surrounding file
style and maintain consistency with the &pm8150l_gpios node formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eec1c86e-6a8b-4647-9349-8b073139c06f
📒 Files selected for processing (6)
config/boards/retroidpocket-rp5.cscconfig/boards/retroidpocket-rpmini.cscpatch/kernel/archive/sm8250-6.18/0070-arm64-dts-qcom-add-SM8250-Retroid-Pocket-variant-Sig.patchpatch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-common.dtsipatch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-rp5.dtspatch/kernel/archive/sm8250-6.18/dt/sm8250-retroidpocket-rpmini.dts
💤 Files with no reviewable changes (1)
- patch/kernel/archive/sm8250-6.18/0070-arm64-dts-qcom-add-SM8250-Retroid-Pocket-variant-Sig.patch
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
1 participant
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.
#9572 moved before as patch added device trees into out of tree device tree files. How this was not done for
current, which is kind of odd because 6.19.y was actually never a target for these devices.Fixes https://forum.armbian.com/topic/59330-compiling-failing-for-sm8250-retroid/#comment-236565
How Has This Been Tested?
./compile.sh BOARD=retroidpocket-rp5 BRANCH=edge BUILD_DESKTOP=yes BUILD_MINIMAL=no DESKTOP_APPGROUPS_SELECTED= DESKTOP_ENVIRONMENT=gnome DESKTOP_ENVIRONMENT_CONFIG_NAME=config_base KERNEL_CONFIGURE=no RELEASE=noble./compile.sh BOARD=retroidpocket-rp5 BRANCH=current BUILD_DESKTOP=yes BUILD_MINIMAL=no DESKTOP_APPGROUPS_SELECTED= DESKTOP_ENVIRONMENT=gnome DESKTOP_ENVIRONMENT_CONFIG_NAME=config_base KERNEL_CONFIGURE=no RELEASE=nobleChecklist:
Summary by CodeRabbit
Chores
Configuration