Orangepi RV2 Board with Spacemit-Famliy both Current and Edge #9299
Conversation
…figs Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Required to compile BCM bluetooth firmware hacking tool. Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
…imit) Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
…LV chip setting) Signed-off-by: Sven-Ola Tuecke <[email protected]>
Note: HW AES is really slow with LUKS2 when using default 512 byte block size, regardless what cryptsetup benchmark prints out (this uses 65536 bytes). Performance is much better with luksFormat --sector-size 4096. Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Added with new extension named bcmdhd-spacemit (hosted on Codeberg.org) Signed-off-by: Sven-Ola Tuecke <[email protected]>
This is for use with fdtoverlays directive in extlinux.conf to enabled different configs for the Opi rv2 pin headers Signed-off-by: Sven-Ola Tuecke <[email protected]>
…ver. Disable U-Boot CONFIG_MMC_UHS_SUPPORT otherwise a fast SD card does not report "1.8 Volt mode switch possible" on kernel init which in turn does not enable SDR104 for my SDXC card. Fast mode ok is visible in the kernel log after booting from SD card: root@orangepirv2:~# dmesg |grep mmc [ 2.198816] mmc0: SDHCI controller on d4280000.sdh [d4280000.sdh] using ADMA [ 2.224576] mmc1: SDHCI controller on d4280800.sdh [d4280800.sdh] using ADMA [ 2.272657] mmc0: set tx_delaycode: 159 [ 2.273950] mmc0: pass window [6 68) [ 2.276301] mmc0: pass window [72 198) [ 2.277591] mmc0: pass window [219 255) [ 2.277599] mmc0: tuning done, use the firstly delay_code:134 [ 2.277611] mmc0: new ultra high speed SDR104 SDXC card at address b36b With UHS already enabled in u-boot, no tuning and no SDR104 mode is switched on in the Linux kernel. Seems to be a side effect of an un-complete mmc_deinit() in u-boot/driver/mmc/mmc.c. With MMC_UHS_SUPPORT disabled, the SD card does report SD_ROCR_S18A with then leads to enter spacemit_sdhci_execute_sw_tuning() and the subsequent switch to SDK104 or probably other 1.8 Volt modes is fine. This is at least true for my OrangePi RV2 board. Signed-off-by: Sven-Ola Tuecke <[email protected]>
Driver for Ethernet/PCI as found with Xunlong/Ky on the OrangePi RV2/R2S source tree. Added as kernel patch since the current spacemit family has conflicting changes with rtl8852bs when enabling EXTRAWIFI=yes Note: on the Xunlong/Ky tree, there is also a realtek_pgtool/ subdir that compiles a pgdrv.ko. B/c this does not have a license indication, is only needed e.g. for MAC address programming of efuse / eeprom, and generally does not look required this is ommited here. For details, you may refer to https://github.com/redchenjs/rtnicpg Signed-off-by: Sven-Ola Tuecke <[email protected]>
Note: have a 128Gb 2030 NVME that returns read errors with Active State PM Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
This reverts commit 99ad088.
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
Signed-off-by: Sven-Ola Tuecke <[email protected]>
📝 WalkthroughWalkthroughThis PR introduces comprehensive support for two Orange Pi boards (R2S and RV2) based on the Spacemit K1-X architecture. It includes board configuration scripts, complete device tree definitions, kernel configuration updates, multiple kernel patches for drivers and subsystems, Bluetooth and WiFi support components, and U-Boot integration. The changes span board metadata, hardware initialization, firmware flashing procedures, and peripheral enablement. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Hey @sven-ola! 👋Thanks for submitting your first pull request to the Armbian project — we're excited to have you contributing! 🧡 If you'd like to stay informed about project updates or collaborate more closely with the team, Also, don’t forget to ⭐ star the repo if you haven’t already — and welcome aboard! 🚀 |
🚫 Missing required board assetsThis PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
Missing items
Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check. |
github-actions
bot
added
size/large
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
patch/kernel/archive/spacemit-6.6/dt/Makefile (1)
9-17: Remove the orphanedk1-x_orangepi-rv2.dtbentry from the Makefile.The reference to
k1-x_orangepi-rv2.dtbon line 9 has no corresponding source file. Onlyx1_orangepi-rv2.dtsexists, which generatesx1_orangepi-rv2.dtb(added on line 17). Thek1-x_orangepi-rv2.dtbentry will fail during kernel compilation. Remove this orphaned entry—it should not coexist with the newx1_orangepi-rv2.dtb.
🤖 Fix all issues with AI agents
In `@extensions/bcmdhd-spacemit.sh`:
- Around line 15-45: The script downloads external .deb files
(pcie_url/sdio_url/usb_url) via chroot_sdcard "wget ..." and then installs via
chroot_sdcard_apt_get_install /tmp/"${file_name}" without integrity checks; add
a verification step after each download and before calling
chroot_sdcard_apt_get_install: fetch the upstream checksum/signature (e.g., a
.sha256 or .asc file provided by the bcmdhd-spacemit-dkms release), verify the
.deb against that checksum (sha256sum) and/or verify a signature with gpg, and
abort (return non‑zero) on mismatch or missing verification; integrate this into
the case branch flow using the same variables (pcie_url/sdio_url/usb_url and
file_name) so the check runs after chroot_sdcard "wget ..." and before
use_clean_environment="yes" chroot_sdcard_apt_get_install /tmp/"${file_name}".
In `@packages/bsp/orangepirv2/brcm_patchram_plus.c`:
- Around line 107-123: This source uses POSIX I/O functions (read, write, close)
and timing functions (usleep, alarm) without including the POSIX header; add
`#include` <unistd.h> near the other system headers so functions like read(),
write(), close(), usleep(), and alarm() have correct prototypes and avoid
implicit-declaration warnings/errors in brcm_patchram_plus.c.
- Around line 617-635: Create and use a robust read helper (e.g., safe_read or
read_all) that wraps read(2) in a loop handling EINTR, returns -1 on error, 0 on
EOF, or the total bytes read on success; replace the ad-hoc read loops in
read_event and the lone read in proc_patchram to call this helper, check its
return value and abort/propagate errors instead of looping forever. In
read_event (function read_event) use the helper to read the 3-byte header,
validate the header result (handle EOF/err), extract len = buffer[2] and
validate that len does not exceed the remaining buffer capacity before reading
the payload with the helper; in proc_patchram use the same helper to read both
header and payload, check the returned byte counts, and reject or handle
short/EOF/error results properly. Ensure all error paths return/cleanup
consistently and do not assume read() will always fulfill the requested length.
- Around line 331-344: In parse_bdaddr, check the return value of sscanf and
ensure it parsed all 6 bytes before writing into hci_write_bd_addr; if sscanf
does not return 6, do not touch hci_write_bd_addr or set bdaddr_flag and instead
return an error (non-zero) so parse_cmd_line can surface usage; only copy
bd_addr into hci_write_bd_addr[4 + i] and set bdaddr_flag = 1 when sscanf
succeeded.
In
`@patch/kernel/archive/spacemit-6.18/015-OrangePi-RV2-fix-internal-audio-activate-headphone-g.patch`:
- Around line 30-41: The new CONFIG_SOC_SPACEMIT block must check and handle
errors from of_get_named_gpio and devm_gpio_request_one and fix the kernel style
for the else placement: after calling of_get_named_gpio for "earphone-sw-gpio"
on i2c_client->dev.of_node, only call devm_gpio_request_one if gpio >= 0,
capture its return value, and on failure call dev_err(&i2c_client->dev, "...:
%d\n", ret) (or dev_warn if you prefer optional) instead of ignoring it; also
change the closing brace/else to follow kernel style (`} else {`) when falling
back to dev_info for "No earphone-sw-gpio".
In
`@patch/kernel/archive/spacemit-6.18/016-Revert-k1x_rproc-avoid-creating-busy-looping-mailbox.patch`:
- Around line 37-57: Rename the misspelled function __process_theread to
__process_thread and update any references/callers accordingly; also fix the
invalid realtime priority by setting param.sched_priority to a valid 1–99 value
(e.g., use sched_get_priority_max(SCHED_FIFO) or at least 1) before calling
sched_setscheduler(current, SCHED_FIFO, ¶m) so the call does not fail with
-EINVAL.
- Around line 121-125: The loop unconditionally calls
kthread_stop(priv->mb[i].mb_thread) which can crash if the thread was never
created (mb_thread is NULL); modify the loop to mirror spacemit_rproc_remove()
by checking the thread state before stopping—e.g., only call kthread_stop when
kthread_running(priv->mb[i].mb_thread) (or priv->mb[i].mb_thread != NULL) is
true—and ensure any associated cleanup such as
mbox_free_channel(priv->mb[i].chan) is performed consistently as in
spacemit_rproc_remove().
In
`@patch/kernel/archive/spacemit-6.18/017-Spacemit-lower-kthread-loop-speed-to-1-sec.patch`:
- Around line 19-23: The SCHED_FIFO call is using an invalid priority
(DEFAULT_PRIO / 0) causing sched_setscheduler to fail; update the sched_param
used in the sched_setscheduler call in this block (struct sched_param param,
reference to DEFAULT_PRIO and sched_setscheduler/current/SCHED_FIFO) to supply a
valid real-time priority (e.g., set sched_priority to 1 for the lowest RT
priority) or alternatively change the scheduling policy from SCHED_FIFO to
SCHED_NORMAL if real-time behavior is not required; ensure the chosen fix
results in a valid range for sched_setscheduler and adjust the param
initialization accordingly.
In `@patch/kernel/archive/spacemit-6.18/dt/x1_orangepi-rv2.dts`:
- Around line 828-836: The node name rgmii0: phy@0 under mdio-bus has a reg =
<0x1> but the unit address (phy@0) does not match the reg; rename the node to
phy@1 to match the reg value and satisfy device tree rules—update the node label
from "phy@0" to "phy@1" (keeping the same properties: compatible, device_type,
reg, phy-mode) so the unit address and reg are consistent.
- Around line 273-289: The eeprom@50 node is marked status = "disabled" but
defines mac_address0 and mac_address1 nvmem cells which eth0 and eth1 reference
via their nvmem-cells; either enable the EEPROM provider by setting eeprom@50 to
status = "okay" (so mac_address0 and mac_address1 become available) or
remove/replace the nvmem-cells references from the ethernet nodes (eth0/eth1) to
point to a valid NVMEM provider/constant instead; locate the eeprom@50 node and
mac_address0/mac_address1 cell labels and update status, or edit the eth0 and
eth1 nodes to remove or retarget their nvmem-cells accordingly.
In
`@patch/kernel/archive/spacemit-6.6/013-Spacemit-copy-kernel-thread-handling-from-Xunlong-Ky.patch`:
- Around line 40-62: Rename the misspelled thread entry function
__process_theread to __process_thread and update any references; also fix the
invalid realtime priority by changing struct sched_param param =
{.sched_priority = 0 } to a valid value (e.g. 1) before calling
sched_setscheduler(current, SCHED_FIFO, ¶m); keep mb->kthread_running,
set_freezable()/try_to_freeze(), wait_for_completion_timeout(&mb->mb_comp, 10)
and the rproc_vq_interrupt(rproc, mb->vq_id) handling as-is, and ensure
sched_setscheduler return is assigned to ret as already done (optionally log on
failure).
In `@patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-r2s.dts`:
- Around line 653-661: The PHY node name and MDIO addresses are inconsistent:
the node rgmii0 is declared as phy@0 but has reg = <0x1>, and both rgmii0 and
rgmii1 share MDIO address 0x1. Fix by renaming the node phy@0 to phy@1 (so the
unit-address matches reg = <0x1>) inside the mdio-bus, and ensure rgmii1 uses a
different reg value (unique MDIO address) than rgmii0 so no two PHYs share the
same MDIO address.
In `@patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts`:
- Around line 271-287: The EEPROM node eeprom@50 is marked status = "disabled"
while eth0/eth1 reference its nvmem-cells mac_address0 and mac_address1, so
enable the provider or relocate the cells: either remove or change status =
"disabled" to status = "okay" on the eeprom@50 node so the NVMEM provider
registers, or move the mac_address0 and mac_address1 child nodes into an enabled
nvmem provider node that eth0/eth1 can reference; ensure the referenced symbols
(eeprom@50, mac_address0, mac_address1, and the eth0/eth1 nvmem-cells usage)
match after the change.
- Around line 826-834: The node name phy@0 under mdio-bus has reg = <0x1>,
causing unit_address_vs_reg warnings; rename the node to phy@1 (matching the reg
value) so the unit address matches reg, i.e., update the node label from "phy@0"
to "phy@1" (retain properties like compatible, device_type, reg, phy-mode and
the rgmii0 label as-is) to eliminate the dtbs_check warning.
- Around line 142-149: The rf-pwrseq node declares a bare pwr-gpios property
which is invalid; either restore the intended GPIO phandle specifier (use
pwr-gpios = <&gpio 67 0>; inside the rf_pwrseq: rf-pwrseq node so the driver can
parse the power-enable line) or remove the pwr-gpios property entirely if that
GPIO is not used; update the rf_pwrseq node accordingly (refer to the rf_pwrseq:
rf-pwrseq node and the commented '<&gpio 67 0>' specifier).
In `@patch/u-boot/legacy/u-boot-spacemit-k1/006-orangepi-rv2-spi-flash.patch`:
- Around line 15-21: The XM25QH128A entry for the spi_nor_ids array is placed
outside the CONFIG_SPI_FLASH_XMC guard so it won't be compiled when only
CONFIG_SPI_FLASH_XMC is set; move the { INFO("XM25QH128A", 0x204118, 0, 64 *
1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) } line into the
`#ifdef` CONFIG_SPI_FLASH_XMC block (or wrap that single entry with `#ifdef`
CONFIG_SPI_FLASH_XMC / `#endif`) so the device ID is included whenever the XMC
driver is enabled and remains excluded otherwise.
In `@patch/u-boot/legacy/u-boot-spacemit-k1/007-orangepi-rv2-r2s-fit.patch`:
- Around line 44-46: The model string in the device tree node for
x1_orangepi-r2s is incorrect—update the model property that currently reads "ky
x1 orangepi-rv2 board" to the correct board name "ky x1 orangepi-r2s board" so
the node for x1_orangepi-r2s reflects the R2S hardware (locate the node with the
model property in x1_orangepi-r2s.dts and replace the string).
🧹 Nitpick comments (5)
patch/kernel/archive/spacemit-6.18/015-OrangePi-RV2-fix-internal-audio-activate-headphone-g.patch (1)
19-22: Consider using the gpiod descriptor-based API.The legacy GPIO API (
of_get_named_gpio,gpio.h,of_gpio.h) is deprecated in the kernel. For 6.18, the preferred approach is to use thegpiod_*descriptor-based API with<linux/gpio/consumer.h>.♻️ Suggested refactor using gpiod API
`#ifdef` CONFIG_SOC_SPACEMIT -#include <linux/gpio.h> -#include <linux/of_gpio.h> +#include <linux/gpio/consumer.h> `#endif`Then in probe:
`#ifdef` CONFIG_SOC_SPACEMIT struct gpio_desc *earphone_gpio; earphone_gpio = devm_gpiod_get_optional(&i2c_client->dev, "earphone-sw", GPIOD_OUT_HIGH); if (IS_ERR(earphone_gpio)) return PTR_ERR(earphone_gpio); `#endif`Note: The DT property would need to change from
earphone-sw-gpiotoearphone-sw-gpiosfor gpiod binding conventions.patch/kernel/archive/spacemit-6.6/013-Spacemit-copy-kernel-thread-handling-from-Xunlong-Ky.patch (1)
54-54: Consider increasing the timeout to reduce polling overhead.A 10-tick timeout results in frequent wakeups (10–100ms depending on HZ). The corresponding 6.18 patch (017) increases this to 1 second via
msecs_to_jiffies(1000). Consider aligning the 6.6 version for consistency.Suggested alignment with 6.18
- wait_for_completion_timeout(&mb->mb_comp, 10); + wait_for_completion_timeout(&mb->mb_comp, msecs_to_jiffies(1000));patch/kernel/archive/spacemit-6.6/overlay/k1-pwm9.dts (1)
10-11: Rename tok1-rpwm9.dtsfor consistency with target node name.The overlay correctly targets
&rpwm9(confirmed in base DT atx1_orangepi-rv2.dts:1219), but the filenamek1-pwm9.dtsis misleading. Rename tok1-rpwm9.dtsto match the target node name, consistent with how other overlays are named (e.g.,k1-pwm7.dtstargets&pwm7).packages/bsp/orangepirv2/orangepirv2-bluetooth.service (1)
5-10: Consider usingType=oneshotfor initialization services.
brcm_patchram_plusperforms a one-time initialization (patchram upload and HCI attachment) then typically exits or continues running to maintain the HCI. The combination ofType=simplewithRemainAfterExit=yesworks butType=oneshotis the more conventional pattern for services that run once to initialize hardware.If
brcm_patchram_pluscontinues running (daemonizes) to keep the UART attached, thenType=simplewithoutRemainAfterExitwould be appropriate. If it exits after initialization,Type=oneshotwithRemainAfterExit=yesis cleaner.patch/u-boot/legacy/u-boot-spacemit-k1/007-orangepi-rv2-r2s-fit.patch (1)
260-262: Minor: Inconsistent indentation.Line 261 (
cd-inverted;) uses spaces instead of tabs, inconsistent with surrounding lines. Same issue at line 602 in the rv2 DTS.
| [[ "${INSTALL_HEADERS}" != "yes" ]] || [[ "${KERNEL_HAS_WORKING_HEADERS}" != "yes" ]] && return 0 | ||
| [[ -z ${BCMDHD_SPACEMIT_TAG} ]] && return 0 | ||
| [[ -z ${BCMDHD_SPACEMIT_TYPE} ]] && return 0 | ||
|
|
||
| local file_name= | ||
| local pcie_url="https://codeberg.org/sven-ola/bcmdhd-spacemit-dkms/releases/download/${BCMDHD_SPACEMIT_TAG}/bcmdhd-spacemit-pcie-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb" | ||
| local sdio_url="https://codeberg.org/sven-ola/bcmdhd-spacemit-dkms/releases/download/${BCMDHD_SPACEMIT_TAG}/bcmdhd-spacemit-sdio-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb" | ||
| local usb_url="https://codeberg.org/sven-ola/bcmdhd-spacemit-dkms/releases/download/${BCMDHD_SPACEMIT_TAG}/bcmdhd-spacemit-usb-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb" | ||
|
|
||
| case "${BCMDHD_SPACEMIT_TYPE}" in | ||
| "pcie") | ||
| file_name=bcmdhd-spacemit-pcie-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb | ||
| use_clean_environment="yes" chroot_sdcard "wget ${pcie_url} -P /tmp" | ||
| ;; | ||
| "sdio") | ||
| file_name=bcmdhd-spacemit-sdio-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb | ||
| use_clean_environment="yes" chroot_sdcard "wget ${sdio_url} -P /tmp" | ||
| ;; | ||
| "usb") | ||
| file_name=bcmdhd-spacemit-usb-dkms_${BCMDHD_SPACEMIT_TAG#v}_all.deb | ||
| use_clean_environment="yes" chroot_sdcard "wget ${usb_url} -P /tmp" | ||
| ;; | ||
| *) | ||
| return 0 | ||
| ;; | ||
| esac | ||
| display_alert "Install bcmdhd-spacemit packages, will build kernel module in chroot" "${EXTENSION}" "info" | ||
| # shellcheck disable=SC2034 | ||
| declare -ag if_error_find_files_sdcard=("/var/lib/dkms/bcmdhd*/*/build/*.log") | ||
| use_clean_environment="yes" chroot_sdcard_apt_get_install /tmp/"${file_name}" | ||
| use_clean_environment="yes" chroot_sdcard "rm -f /tmp/bcmdhd*.deb" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
bcmdhd-spacemit-dkms Codeberg releases checksums signatures
💡 Result:
Codeberg (Forgejo) doesn’t automatically publish checksums or cryptographic signatures for “Releases” assets—a release is essentially a Git tag plus arbitrary attached files (binaries, tarballs, .sha256, .asc, etc.) and release notes. (docs.codeberg.org)
So for bcmdhd-spacemit-dkms on Codeberg, if you want checksums/signatures, you typically need to look for (or the maintainer needs to upload):
- a checksum file alongside the asset (e.g.,
SHA256SUMS,*.sha256) - a detached OpenPGP signature (e.g.,
*.asc) for the asset and/or for the checksum file
Separately, Codeberg supports GPG/SSH verification for Git objects (commits/tags) once the maintainer uploads a key—so a signed tag can provide authenticity for the source at that tag, but it’s distinct from signing the uploaded release binaries themselves. (docs.codeberg.org)
Citations:
- 1: https://docs.codeberg.org/git/using-tags/?utm_source=openai
- 2: https://docs.codeberg.org/security/gpg-key/?utm_source=openai
🏁 Script executed:
# Let's examine the full file to understand the context better
cat -n extensions/bcmdhd-spacemit.shRepository: armbian/build
Length of output: 2538
Add integrity verification before installing external .deb packages
Lines 27, 31, and 35 download and install .deb files from Codeberg releases without checksum or signature verification. This creates a supply-chain risk. Before installation, either:
- Verify the downloaded package against checksums provided by the upstream project (if available), or
- Request that the bcmdhd-spacemit-dkms maintainer publish checksums/signatures alongside releases.
Note that apt-get install on a local .deb file (line 44) does not perform package signature verification—this verification must happen before or during the download.
🤖 Prompt for AI Agents
In `@extensions/bcmdhd-spacemit.sh` around lines 15 - 45, The script downloads
external .deb files (pcie_url/sdio_url/usb_url) via chroot_sdcard "wget ..." and
then installs via chroot_sdcard_apt_get_install /tmp/"${file_name}" without
integrity checks; add a verification step after each download and before calling
chroot_sdcard_apt_get_install: fetch the upstream checksum/signature (e.g., a
.sha256 or .asc file provided by the bcmdhd-spacemit-dkms release), verify the
.deb against that checksum (sha256sum) and/or verify a signature with gpg, and
abort (return non‑zero) on mismatch or missing verification; integrate this into
the case branch flow using the same variables (pcie_url/sdio_url/usb_url and
file_name) so the check runs after chroot_sdcard "wget ..." and before
use_clean_environment="yes" chroot_sdcard_apt_get_install /tmp/"${file_name}".
| #include <stdio.h> | ||
| #include <getopt.h> | ||
| #include <errno.h> | ||
|
|
||
| #include <sys/types.h> | ||
| #include <sys/stat.h> | ||
| #include <fcntl.h> | ||
|
|
||
| #include <stdlib.h> | ||
|
|
||
| #ifdef ANDROID | ||
| #include <termios.h> | ||
| #else | ||
| #include <sys/termios.h> | ||
| #include <sys/ioctl.h> | ||
| #include <limits.h> | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "brcm_patchram_plus.c" -type fRepository: armbian/build
Length of output: 104
🏁 Script executed:
sed -n '107,130p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 443
🏁 Script executed:
grep -n "unistd.h" ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 39
🏁 Script executed:
grep -nE "(read|write|close|usleep|sleep|alarm)\s*\(" ./packages/bsp/orangepirv2/brcm_patchram_plus.c | head -20Repository: armbian/build
Length of output: 740
🏁 Script executed:
# Check what headers might be including unistd.h indirectly
# Also check the C standard to see if unistd.h is required
# First, let's see how this file is compiled (look for build files)
find . -name "*.mk" -o -name "Makefile" -o -name "*.cmake" | grep -i orangeRepository: armbian/build
Length of output: 39
🏁 Script executed:
# Verify the context around the proposed fix location
sed -n '110,115p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 142
Add <unistd.h> for POSIX I/O prototypes
This file uses read(), write(), close(), usleep(), and alarm() without including <unistd.h>, which will cause implicit function declaration warnings or build failures with strict C flags.
🛠️ Proposed fix
`#include` <stdlib.h>
+#include <unistd.h>
`#ifdef` ANDROID
`#include` <termios.h>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <stdio.h> | |
| #include <getopt.h> | |
| #include <errno.h> | |
| #include <sys/types.h> | |
| #include <sys/stat.h> | |
| #include <fcntl.h> | |
| #include <stdlib.h> | |
| #ifdef ANDROID | |
| #include <termios.h> | |
| #else | |
| #include <sys/termios.h> | |
| #include <sys/ioctl.h> | |
| #include <limits.h> | |
| #endif | |
| `#include` <stdio.h> | |
| `#include` <getopt.h> | |
| `#include` <errno.h> | |
| `#include` <sys/types.h> | |
| `#include` <sys/stat.h> | |
| `#include` <fcntl.h> | |
| `#include` <stdlib.h> | |
| `#include` <unistd.h> | |
| `#ifdef` ANDROID | |
| `#include` <termios.h> | |
| `#else` | |
| `#include` <sys/termios.h> | |
| `#include` <sys/ioctl.h> | |
| `#include` <limits.h> | |
| `#endif` |
🧰 Tools
🪛 Clang (14.0.6)
[error] 107-107: 'stdio.h' file not found
(clang-diagnostic-error)
🤖 Prompt for AI Agents
In `@packages/bsp/orangepirv2/brcm_patchram_plus.c` around lines 107 - 123, This
source uses POSIX I/O functions (read, write, close) and timing functions
(usleep, alarm) without including the POSIX header; add `#include` <unistd.h> near
the other system headers so functions like read(), write(), close(), usleep(),
and alarm() have correct prototypes and avoid implicit-declaration
warnings/errors in brcm_patchram_plus.c.
| parse_bdaddr(char *optarg) | ||
| { | ||
| int bd_addr[6]; | ||
| int i; | ||
|
|
||
| sscanf(optarg, "%02X:%02X:%02X:%02X:%02X:%02X", | ||
| &bd_addr[5], &bd_addr[4], &bd_addr[3], | ||
| &bd_addr[2], &bd_addr[1], &bd_addr[0]); | ||
|
|
||
| for (i = 0; i < 6; i++) { | ||
| hci_write_bd_addr[4 + i] = bd_addr[i]; | ||
| } | ||
|
|
||
| bdaddr_flag = 1; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "brcm_patchram_plus.c" -type fRepository: armbian/build
Length of output: 104
🏁 Script executed:
wc -l ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 108
🏁 Script executed:
sed -n '325,355p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 442
🏁 Script executed:
rg "parse_cmd_line|parse_bdaddr" ./packages/bsp/orangepirv2/brcm_patchram_plus.c -A 2 -B 2Repository: armbian/build
Length of output: 461
🏁 Script executed:
sed -n '660,750p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 1445
🏁 Script executed:
rg "hci_write_bd_addr" ./packages/bsp/orangepirv2/brcm_patchram_plus.c -B 3 -A 3Repository: armbian/build
Length of output: 558
🏁 Script executed:
sed -n '400,500p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 3001
🏁 Script executed:
sed -n '500,600p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 1969
🏁 Script executed:
sed -n '330,350p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 389
🏁 Script executed:
sed -n '360,430p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 992
Validate BD_ADDR parsing result
If sscanf doesn't parse all 6 bytes, the code writes uninitialized values into the HCI command buffer. Return an error to allow parse_cmd_line to surface usage.
Proposed fix
- sscanf(optarg, "%02X:%02X:%02X:%02X:%02X:%02X",
- &bd_addr[5], &bd_addr[4], &bd_addr[3],
- &bd_addr[2], &bd_addr[1], &bd_addr[0]);
+ if (sscanf(optarg, "%02X:%02X:%02X:%02X:%02X:%02X",
+ &bd_addr[5], &bd_addr[4], &bd_addr[3],
+ &bd_addr[2], &bd_addr[1], &bd_addr[0]) != 6) {
+ fprintf(stderr, "invalid bd_addr: %s\n", optarg);
+ return 1;
+ }🤖 Prompt for AI Agents
In `@packages/bsp/orangepirv2/brcm_patchram_plus.c` around lines 331 - 344, In
parse_bdaddr, check the return value of sscanf and ensure it parsed all 6 bytes
before writing into hci_write_bd_addr; if sscanf does not return 6, do not touch
hci_write_bd_addr or set bdaddr_flag and instead return an error (non-zero) so
parse_cmd_line can surface usage; only copy bd_addr into hci_write_bd_addr[4 +
i] and set bdaddr_flag = 1 when sscanf succeeded.
| read_event(int fd, uchar *buffer) | ||
| { | ||
| int i = 0; | ||
| int len = 3; | ||
| int count; | ||
|
|
||
| while ((count = read(fd, &buffer[i], len)) < len) { | ||
| i += count; | ||
| len -= count; | ||
| } | ||
|
|
||
| i += count; | ||
| len = buffer[2]; | ||
|
|
||
| while ((count = read(fd, &buffer[i], len)) < len) { | ||
| i += count; | ||
| len -= count; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "brcm_patchram_plus.c" -type fRepository: armbian/build
Length of output: 104
🏁 Script executed:
wc -l ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 108
🏁 Script executed:
sed -n '617,635p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 337
🏁 Script executed:
sed -n '694,705p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 247
🏁 Script executed:
rg -n "read_fully|ssize_t.*read" ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 39
🏁 Script executed:
sed -n '680,710p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 602
🏁 Script executed:
sed -n '650,695p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 620
🏁 Script executed:
rg -n "errno|perror|EINTR" ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 375
🏁 Script executed:
sed -n '806,820p' ./packages/bsp/orangepirv2/brcm_patchram_plus.cRepository: armbian/build
Length of output: 407
Handle short reads and errors in read_event and proc_patchram
Both read_event (lines 617-635) and proc_patchram (lines 694-705) have dangerous error-handling gaps:
read_eventassumesread()always returns the requested length; if it returns 0 (EOF) or -1 (EINTR/error), the conditioncount < lennever becomes false, causing an infinite loop.proc_patchramignores the return value ofread()at line 699 entirely, and lacks buffer size validation for thelenvariable read from the file.
The codebase already demonstrates proper error handling elsewhere (e.g., lines 813–820), but this pattern is missing here. Implement a robust read helper that handles EOF, EINTR, and other errors, and apply it to both functions.
🤖 Prompt for AI Agents
In `@packages/bsp/orangepirv2/brcm_patchram_plus.c` around lines 617 - 635, Create
and use a robust read helper (e.g., safe_read or read_all) that wraps read(2) in
a loop handling EINTR, returns -1 on error, 0 on EOF, or the total bytes read on
success; replace the ad-hoc read loops in read_event and the lone read in
proc_patchram to call this helper, check its return value and abort/propagate
errors instead of looping forever. In read_event (function read_event) use the
helper to read the 3-byte header, validate the header result (handle EOF/err),
extract len = buffer[2] and validate that len does not exceed the remaining
buffer capacity before reading the payload with the helper; in proc_patchram use
the same helper to read both header and payload, check the returned byte counts,
and reject or handle short/EOF/error results properly. Ensure all error paths
return/cleanup consistently and do not assume read() will always fulfill the
requested length.
| +#ifdef CONFIG_SOC_SPACEMIT | ||
| + { | ||
| + int gpio = of_get_named_gpio(i2c_client->dev.of_node, "earphone-sw-gpio", 0); | ||
| + if (0 <= gpio) { | ||
| + devm_gpio_request_one(&i2c_client->dev, gpio, | ||
| + GPIOF_OUT_INIT_HIGH, NULL); | ||
| + } | ||
| + else { | ||
| + dev_info(&i2c_client->dev, "No earphone-sw-gpio\n"); | ||
| + } | ||
| + } | ||
| +#endif |
There was a problem hiding this comment.
Missing error handling and kernel coding style violation.
-
The return value of
devm_gpio_request_one()is not checked. While this GPIO appears optional, silently ignoring failures may mask configuration issues. -
The
elseplacement on line 37 violates kernel coding style (should be} else {on same line).
🔧 Proposed fix
`#ifdef` CONFIG_SOC_SPACEMIT
- {
- int gpio = of_get_named_gpio(i2c_client->dev.of_node, "earphone-sw-gpio", 0);
- if (0 <= gpio) {
- devm_gpio_request_one(&i2c_client->dev, gpio,
- GPIOF_OUT_INIT_HIGH, NULL);
- }
- else {
- dev_info(&i2c_client->dev, "No earphone-sw-gpio\n");
- }
+ {
+ int gpio = of_get_named_gpio(i2c_client->dev.of_node, "earphone-sw-gpio", 0);
+ if (gpio >= 0) {
+ int ret = devm_gpio_request_one(&i2c_client->dev, gpio,
+ GPIOF_OUT_INIT_HIGH, "earphone-sw");
+ if (ret)
+ dev_warn(&i2c_client->dev, "Failed to request earphone-sw-gpio: %d\n", ret);
+ } else {
+ dev_info(&i2c_client->dev, "No earphone-sw-gpio\n");
+ }
}
`#endif`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +#ifdef CONFIG_SOC_SPACEMIT | |
| + { | |
| + int gpio = of_get_named_gpio(i2c_client->dev.of_node, "earphone-sw-gpio", 0); | |
| + if (0 <= gpio) { | |
| + devm_gpio_request_one(&i2c_client->dev, gpio, | |
| + GPIOF_OUT_INIT_HIGH, NULL); | |
| + } | |
| + else { | |
| + dev_info(&i2c_client->dev, "No earphone-sw-gpio\n"); | |
| + } | |
| + } | |
| +#endif | |
| `#ifdef` CONFIG_SOC_SPACEMIT | |
| { | |
| int gpio = of_get_named_gpio(i2c_client->dev.of_node, "earphone-sw-gpio", 0); | |
| if (gpio >= 0) { | |
| int ret = devm_gpio_request_one(&i2c_client->dev, gpio, | |
| GPIOF_OUT_INIT_HIGH, "earphone-sw"); | |
| if (ret) | |
| dev_warn(&i2c_client->dev, "Failed to request earphone-sw-gpio: %d\n", ret); | |
| } else { | |
| dev_info(&i2c_client->dev, "No earphone-sw-gpio\n"); | |
| } | |
| } | |
| `#endif` |
🤖 Prompt for AI Agents
In
`@patch/kernel/archive/spacemit-6.18/015-OrangePi-RV2-fix-internal-audio-activate-headphone-g.patch`
around lines 30 - 41, The new CONFIG_SOC_SPACEMIT block must check and handle
errors from of_get_named_gpio and devm_gpio_request_one and fix the kernel style
for the else placement: after calling of_get_named_gpio for "earphone-sw-gpio"
on i2c_client->dev.of_node, only call devm_gpio_request_one if gpio >= 0,
capture its return value, and on failure call dev_err(&i2c_client->dev, "...:
%d\n", ret) (or dev_warn if you prefer optional) instead of ignoring it; also
change the closing brace/else to follow kernel style (`} else {`) when falling
back to dev_info for "No earphone-sw-gpio".
| rf_pwrseq: rf-pwrseq { | ||
| compatible = "spacemit,rf-pwrseq"; | ||
| //vdd-supply = <&ldo_7>; | ||
| //vdd_voltage = <3300000>; | ||
| io-supply = <&dcdc_3>; | ||
| io_voltage = <1800000>; | ||
| pwr-gpios;// = <&gpio 67 0>; | ||
| status = "okay"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the target file to see the current state of the rf_pwrseq node
cat -n patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | sed -n '140,160p'Repository: armbian/build
Length of output: 701
🏁 Script executed:
# Also check the other issues mentioned in the scratchpad at lines 271-287 and 826-834
cat -n patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | sed -n '269,290p'Repository: armbian/build
Length of output: 568
🏁 Script executed:
cat -n patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | sed -n '824,840p'Repository: armbian/build
Length of output: 471
🏁 Script executed:
# Let's look for any device tree binding docs or comments about rf-pwrseq in the codebase
find . -type f \( -name "*.txt" -o -name "*.yaml" -o -name "*.md" \) | xargs grep -l "rf-pwrseq" 2>/dev/null | head -5Repository: armbian/build
Length of output: 39
🏁 Script executed:
# Search for how *-gpios properties are typically used in this device tree file
rg "gpios\s*=" patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | head -10Repository: armbian/build
Length of output: 465
🏁 Script executed:
# Search for spacemit,rf-pwrseq binding documentation
find . -type f \( -name "*.txt" -o -name "*.yaml" -o -name "*.md" \) -path "*/bindings/*" | xargs grep -l "rf-pwrseq\|spacemit" 2>/dev/null | head -10Repository: armbian/build
Length of output: 39
🏁 Script executed:
# Search for any kernel driver code that handles rf-pwrseq
find . -type f \( -name "*.c" -o -name "*.h" \) | xargs grep -l "rf.pwrseq\|rf_pwrseq" 2>/dev/null | head -5Repository: armbian/build
Length of output: 39
🏁 Script executed:
# Look for device tree validation or binding examples in the codebase
rg "pwr-gpios" --type yaml --type txt 2>/dev/null | head -20Repository: armbian/build
Length of output: 39
🏁 Script executed:
# Check if there are other similar pwrseq implementations in the tree for reference
rg "pwrseq.*compatible" patch/kernel/archive/spacemit-6.6/dt/ --type dts 2>/dev/nullRepository: armbian/build
Length of output: 39
🌐 Web query:
device tree gpio properties specification format requirements
💡 Result:
GPIO properties in Device Tree: format & requirements
1) Consumer-side GPIO properties (*-gpios)
- GPIOs are described in consumer nodes using one or more properties named
[<function>-]gpios(preferred). Older[<function>-]gpiois deprecated for new bindings. [1] - The property value is a phandle-array made of one or more entries of the form:
<&gpio_controller ...gpio-specifier-cells...>, where the number of specifier cells is defined by the controller’s#gpio-cells. [1] - Multiple GPIOs in one property are discouraged unless they serve the same function (e.g., a parallel bus). Otherwise use separate, clearly named properties (e.g.,
reset-gpios,enable-gpios). [1]
Canonical example
reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
Here &gpio1 is the GPIO controller phandle; 12 and GPIO_ACTIVE_LOW are the controller-defined specifier cells (common convention: line offset + flags). [1]
2) GPIO controller node requirements
A node that provides GPIOs (a GPIO controller) must contain: [1]
gpio-controller;(empty boolean property)#gpio-cells = <N>;(how many cells follow the phandle in each GPIO specifier)
Common optional controller properties include ngpios (how many lines are actually usable) and mapping-related properties like gpio-ranges. [1]
3) Specifier cell conventions / flags
- The meaning of the GPIO specifier cells is controller-specific, but Linux strongly recommends a 2-cell form:
- local GPIO line offset/number
- consumer flags bitfield (polarity, open-drain, etc.) [1]
- When the last cell is a generic flags bitfield, use the standard macros from
include/dt-bindings/gpio/gpio.h(e.g.,GPIO_ACTIVE_LOW,GPIO_ACTIVE_HIGH). [1][2]
4) What a “phandle” is (background)
- A phandle is a unique numeric identifier for a node, used by other nodes to reference it (DTS source typically uses
&labelwhich compiles to a phandle). [3]
Sources: Linux GPIO devicetree binding (Documentation/devicetree/bindings/gpio/gpio.txt) for naming/format/controller requirements and recommended practices [1], Linux kernel GPIO DT mapping overview [2], Devicetree Specification basics for phandle semantics [3].
References
[1] (git.ti.com)
[2] (kernel.org)
[3] (devicetree-specification.readthedocs.io)
Citations:
- 1: https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/Documentation/devicetree/bindings/gpio/gpio.txt?utm_source=openai
- 2: https://www.kernel.org/doc/html/latest/driver-api/gpio/board.html?utm_source=openai
- 3: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html?utm_source=openai
🌐 Web query:
Linux device tree *-gpios property bare empty value invalid
💡 Result:
In Linux devicetree, a *-gpios property is (by convention and in most bindings) a GPIO specifier list, i.e. a phandle-array like:
reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
A bare/empty property:
reset-gpios;
is therefore invalid for *-gpios, because an empty value is only meaningful for boolean “flag” properties (presence/absence conveys true/false). (devicetree-specification.readthedocs.io) The Linux DT binding schema examples likewise treat foo-gpios as a property that carries items (not a boolean). (docs.kernel.org)
What to do instead:
- If the GPIO is optional: omit the
*-gpiosproperty entirely (and ensure the driver/binding defines it as optional). - If you need to express “no GPIO connected” while keeping the property: that generally requires a binding-defined convention (driver-specific). The generic DT rules don’t define an “empty gpios means none” encoding—so check the specific binding/driver and follow what it documents.
If you share the exact property name (e.g., reset-gpios, enable-gpios) and the device/binding, I can tell you the correct representation for that specific case.
Citations:
- 1: https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html?utm_source=openai
- 2: https://docs.kernel.org/5.15/devicetree/bindings/writing-schema.html?utm_source=openai
pwr-gpios property is missing required GPIO specifier
The bare pwr-gpios; declaration violates the device tree specification: *-gpios properties must contain phandle-array GPIO specifiers (<&gpio line flags>), not empty values. This will prevent proper GPIO parsing and cause the rf-pwrseq driver to fail probing. The commented value suggests the intended spec was <&gpio 67 0>, so either restore it or remove the property if unused.
🛠️ Proposed fix (if GPIO67 is the intended power-enable line)
- pwr-gpios;// = <&gpio 67 0>;
+ pwr-gpios = <&gpio 67 GPIO_ACTIVE_HIGH>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rf_pwrseq: rf-pwrseq { | |
| compatible = "spacemit,rf-pwrseq"; | |
| //vdd-supply = <&ldo_7>; | |
| //vdd_voltage = <3300000>; | |
| io-supply = <&dcdc_3>; | |
| io_voltage = <1800000>; | |
| pwr-gpios;// = <&gpio 67 0>; | |
| status = "okay"; | |
| rf_pwrseq: rf-pwrseq { | |
| compatible = "spacemit,rf-pwrseq"; | |
| //vdd-supply = <&ldo_7>; | |
| //vdd_voltage = <3300000>; | |
| io-supply = <&dcdc_3>; | |
| io_voltage = <1800000>; | |
| pwr-gpios = <&gpio 67 GPIO_ACTIVE_HIGH>; | |
| status = "okay"; |
🤖 Prompt for AI Agents
In `@patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts` around lines 142 -
149, The rf-pwrseq node declares a bare pwr-gpios property which is invalid;
either restore the intended GPIO phandle specifier (use pwr-gpios = <&gpio 67
0>; inside the rf_pwrseq: rf-pwrseq node so the driver can parse the
power-enable line) or remove the pwr-gpios property entirely if that GPIO is not
used; update the rf_pwrseq node accordingly (refer to the rf_pwrseq: rf-pwrseq
node and the commented '<&gpio 67 0>' specifier).
| eeprom@50{ | ||
| compatible = "atmel,24c02"; | ||
| reg = <0x50>; | ||
| #address-cells = <1>; | ||
| #size-cells = <1>; | ||
|
|
||
| power-domains = <&power K1X_PMU_DUMMY_PWR_DOMAIN>; | ||
| status = "disabled"; | ||
|
|
||
| mac_address0: mac_address0@0 { | ||
| reg = <0x0 6>; | ||
| }; | ||
|
|
||
| mac_address1: mac_address1@6 { | ||
| reg = <0x6 6>; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | head -400 | tail -250Repository: armbian/build
Length of output: 4958
🏁 Script executed:
# Search for eth0 and eth1 definitions in the file
rg "eth0|eth1" patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts -A 5 -B 2Repository: armbian/build
Length of output: 319
🏁 Script executed:
# Search for references to mac_address0 or mac_address1
rg "mac_address" patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts -B 2 -A 2Repository: armbian/build
Length of output: 390
🏁 Script executed:
# Get the full context around the eeprom section (lines 271-287)
sed -n '260,310p' patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dtsRepository: armbian/build
Length of output: 1167
EEPROM disabled while eth0/eth1 reference its MAC address cells
Both eth0 and eth1 use nvmem-cells to reference mac_address0 and mac_address1 from the EEPROM, but the EEPROM node has status = "disabled". This prevents NVMEM provider registration, causing MAC address resolution to fail and the NICs to fall back to random MACs. Either enable the EEPROM or move the MAC cell definitions to an enabled provider.
🤖 Prompt for AI Agents
In `@patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts` around lines 271 -
287, The EEPROM node eeprom@50 is marked status = "disabled" while eth0/eth1
reference its nvmem-cells mac_address0 and mac_address1, so enable the provider
or relocate the cells: either remove or change status = "disabled" to status =
"okay" on the eeprom@50 node so the NVMEM provider registers, or move the
mac_address0 and mac_address1 child nodes into an enabled nvmem provider node
that eth0/eth1 can reference; ensure the referenced symbols (eeprom@50,
mac_address0, mac_address1, and the eth0/eth1 nvmem-cells usage) match after the
change.
| mdio-bus { | ||
| #address-cells = <0x1>; | ||
| #size-cells = <0x0>; | ||
| rgmii0: phy@0 { | ||
| compatible = "ethernet-phy-ieee802.3-c22"; | ||
| device_type = "ethernet-phy"; | ||
| reg = <0x1>; | ||
| phy-mode = "rgmii"; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
fd -t f "x1_orangepi-rv2.dts" --exec wc -l {} \;Repository: armbian/build
Length of output: 185
🏁 Script executed:
# Find and read the specific file
find . -name "x1_orangepi-rv2.dts" -type fRepository: armbian/build
Length of output: 175
🏁 Script executed:
# Read the specific section mentioned in the review
sed -n '820,840p' ./patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts | cat -nRepository: armbian/build
Length of output: 561
🏁 Script executed:
# Search for any references to phy@0 or rgmii0 in the spacemit-6.6 device tree file
rg "phy@0|rgmii0" ./patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dtsRepository: armbian/build
Length of output: 99
🏁 Script executed:
# Also check if there are similar issues in the spacemit-6.18 version
sed -n '820,840p' ./patch/kernel/archive/spacemit-6.18/dt/x1_orangepi-rv2.dts | cat -nRepository: armbian/build
Length of output: 535
PHY unit-address should match reg
phy@0 uses reg = <0x1>, which triggers unit_address_vs_reg warnings in dtbs_check. Rename the node to match the address.
🛠️ Proposed fix
- rgmii0: phy@0 {
+ rgmii0: phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
device_type = "ethernet-phy";
reg = <0x1>;
phy-mode = "rgmii";
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mdio-bus { | |
| #address-cells = <0x1>; | |
| #size-cells = <0x0>; | |
| rgmii0: phy@0 { | |
| compatible = "ethernet-phy-ieee802.3-c22"; | |
| device_type = "ethernet-phy"; | |
| reg = <0x1>; | |
| phy-mode = "rgmii"; | |
| }; | |
| mdio-bus { | |
| `#address-cells` = <0x1>; | |
| `#size-cells` = <0x0>; | |
| rgmii0: phy@1 { | |
| compatible = "ethernet-phy-ieee802.3-c22"; | |
| device_type = "ethernet-phy"; | |
| reg = <0x1>; | |
| phy-mode = "rgmii"; | |
| }; |
🤖 Prompt for AI Agents
In `@patch/kernel/archive/spacemit-6.6/dt/x1_orangepi-rv2.dts` around lines 826 -
834, The node name phy@0 under mdio-bus has reg = <0x1>, causing
unit_address_vs_reg warnings; rename the node to phy@1 (matching the reg value)
so the unit address matches reg, i.e., update the node label from "phy@0" to
"phy@1" (retain properties like compatible, device_type, reg, phy-mode and the
rgmii0 label as-is) to eliminate the dtbs_check warning.
| @@ -444,6 +444,7 @@ const struct flash_info spi_nor_ids[] = { | ||
| { INFO("w25m512jw", 0xef6119, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, | ||
| { INFO("w25m512jv", 0xef7119, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, | ||
| { INFO("w25h02jv", 0xef9022, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, | ||
| + { INFO("XM25QH128A", 0x204118, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, | ||
| #endif | ||
| #ifdef CONFIG_SPI_FLASH_XMC |
There was a problem hiding this comment.
XM25QH128A is under the wrong config guard.
It’s inserted before the #ifdef CONFIG_SPI_FLASH_XMC block, so it will be compiled only if the preceding vendor option is enabled. If the intent is XMC, the ID can be missing when only CONFIG_SPI_FLASH_XMC is set.
🐛 Proposed fix
- { INFO("XM25QH128A", 0x204118, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
-#endif
-#ifdef CONFIG_SPI_FLASH_XMC
+#endif
+#ifdef CONFIG_SPI_FLASH_XMC
+ { INFO("XM25QH128A", 0x204118, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },🤖 Prompt for AI Agents
In `@patch/u-boot/legacy/u-boot-spacemit-k1/006-orangepi-rv2-spi-flash.patch`
around lines 15 - 21, The XM25QH128A entry for the spi_nor_ids array is placed
outside the CONFIG_SPI_FLASH_XMC guard so it won't be compiled when only
CONFIG_SPI_FLASH_XMC is set; move the { INFO("XM25QH128A", 0x204118, 0, 64 *
1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) } line into the
`#ifdef` CONFIG_SPI_FLASH_XMC block (or wrap that single entry with `#ifdef`
CONFIG_SPI_FLASH_XMC / `#endif`) so the device ID is included whenever the XMC
driver is enabled and remains excluded otherwise.
| +/ { | ||
| + model = "ky x1 orangepi-rv2 board"; | ||
| + |
There was a problem hiding this comment.
Model string appears to be a copy-paste error.
The x1_orangepi-r2s.dts file has model = "ky x1 orangepi-rv2 board" but this is the R2S board, not RV2. The model string should likely be "ky x1 orangepi-r2s board" to distinguish the two boards.
🐛 Proposed fix
/ {
- model = "ky x1 orangepi-rv2 board";
+ model = "ky x1 orangepi-r2s board";🤖 Prompt for AI Agents
In `@patch/u-boot/legacy/u-boot-spacemit-k1/007-orangepi-rv2-r2s-fit.patch` around
lines 44 - 46, The model string in the device tree node for x1_orangepi-r2s is
incorrect—update the model property that currently reads "ky x1 orangepi-rv2
board" to the correct board name "ky x1 orangepi-r2s board" so the node for
x1_orangepi-r2s reflects the R2S hardware (locate the node with the model
property in x1_orangepi-r2s.dts and replace the string).
|
R2S board photo |
sven-ola
changed the title
The orangepirv2.png image was taken from my board. I use it with an UART (wires cut in photo) and a Samsung NVME 128 Gb SSD. The orangepir2s.png is for reference only. I do not own this board, but I asked at the OpenWrt forum for a photo. Github member https://github.com/peterwillcn was so kind and add this photo of his R2S board to the merge RQ, see armbian/build#9299 (comment) Signed-off-by: Sven-Ola Tuecke <[email protected]>
|
Thanks to @peterwillcn for making this photo of his OrangePi R2S available for this PR. I use it together with photo from my board to create the necessary pull RQ, see armbian/armbian.github.io#201 |
2 participants
Description
This PR supports a new board: Xunlong Orange Pi RV2. I already applied for maintainership for this board, but currently the board configs are *.wip. Note, that the smaller sibling, the Orange Pi R2V is also supported, but I do not own the HW, thus it's unsupported probably.
Documentation summary for feature / change
You can read about this PR in the Armbian forum, see the respective thread under https://forum.armbian.com/topic/56846-orange-pi-rv2/ I also generated and uploaded testing images built with this on my private project website (see https://privat-in.de/ goto downloads): current-trixie-minimal, edge-trixie-minimal, and edge-trixie-desktop-mate.
Here is a list of features (valid both for 6.6/current and for 6.18/edge if not noted otherwise)
How Has This Been Tested?
This was tested with the images one can download. I have a number of people that tried my images and provided feedback (see Forum thread linked above).
Notes
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.