Radxa Rock-s0 patches consolidation #8050
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| */ | ||
| + /* | ||
| + * Armbian: this has been commented out due to consolidation of this patch: | ||
| + * https://github.com/paolosabatino/armbian-build/blob/935149d2822cd2a2c63891ca6614563f5538b706/patch/kernel/archive/rockchip64-6.12/board-rocks0-0002-Revert-arm64-dts-rockchip-Fix-sdmmc-access-on-rk3308.patch |
There was a problem hiding this comment.
@paolosabatino there should not be any need to comment out this, the revert was probably just to fix build due to the hefty modification going on.
| vin-supply = <&vcc5v0_sys>; | ||
| }; | ||
|
|
||
| + vcc5v0_otg: vcc5v0-otg { |
There was a problem hiding this comment.
@paolosabatino the GPIO0_C5/OTG_DRVBUS_d is not hooked up to anything on the ROCK S0 according to the schematics, so there is no need to add this.
| }; | ||
|
|
||
| +&i2c1 { | ||
| + status = "okay"; |
There was a problem hiding this comment.
i2c1 is hooked up to the gpio pins and there should not be any need to enable it without an dt-overlay
|
Please hold off on this until I've had a chance to test on actual hardware.
/boot/dtb/rockchip/overlay: |
|
I just verified that, with this patch, the older Rock-S0 board DOES boot from SDMMC. However it works now, it does hold the required GPIO4_D6 low. The WiFi requires that an external antenna is connected. What was wrong with the previous set up, where the kernel defaulted to using the internal antenna until the rk3308-s0-ext-antenna.dtbo was specified in armbianEnv.txt ? Why did this even need to change? Although the internal antenna is not great, it is always available, so it probably should be the default configuration. I like the fact that the pcm512a codec now requires its own overlay dtbo to enable. I'll approve this once the antenna issue is sorted. |
|
|
||
| +&codec { | ||
| + status = "okay"; | ||
| + #sound-dai-cells = <0>; |
There was a problem hiding this comment.
Normally the status property should be the last property, followed by any child nodes.
Also the #sound-dai-cells should probably have been added to the codec node in the soc dtsi.
| + | ||
| +&i2s_8ch_2 { | ||
| + status = "okay"; | ||
| + #sound-dai-cells = <0>; |
There was a problem hiding this comment.
Normally the status property should be the last property, followed by any child nodes.
Also the #sound-dai-cells should probably have been added to the codec node in the soc dtsi.
There is a OM8751D (SPDT) controlled by two GPIO ( This could possible be modeled as |
|
Thanks for the feedback; I move this PR to draft, since there is more work to do. Currently I have some personal life issues, I'll address this when I will sort them out. |
* remove the patches that are shredding upstream device tree * consolidation into single patch that apply over upstream dt * add pcm5102a device tree overlay for handy usage
fffd224 to
82ead71
Compare
Description
In order to tidy up (see this PR), Radxa Rock-s0 board patch has been revised to work with the mainline device tree, and avoid shred it. In case of possibile conflicts with mainline kernel, it will be easier to deal with.
@brentr I could not test directly the board, since I don't have it. I built a rockpi-s image, it works and dmesg is clean, but you have to tell if rock-s0 works ok.
Some other patches have been removed, as @Kwiboo suggested in the mentioned PR and refences to sound codec pcm5102a have been removed from the board device tree (there is no such codec mentioned in the official specs, is there?) and moved into
rk3308-pcm5102a.dtbodevice tree overlay.The internal/external antenna juggling has been left out for the moment, I need a bit of clarification, as I see there are two GPIOs to power each antenna, but what happens if both are pulled down? I guess it could be easier to handle with just a pinctrl definition rather than a
regulator-fixedentry.This is only for rockchip64-edge 6.14 kernel, if it works can be backported to current 6.12 kernel; @brentr will decide what is best.
How Has This Been Tested?
Checklist: