QCS6490 family & radxa-dragon-q6a improvements#9289
QCS6490 family & radxa-dragon-q6a improvements#9289igorpecovnik merged 1 commit intoarmbian:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR modifies board and kernel configuration for the Radxa Dragon Q6A platform, transitions from edge to current kernel branch with 6.18.2 versions, and removes/reverts numerous kernel patches affecting audio, firmware, display, device trees, and hardware drivers while introducing a comprehensive kernel defconfig. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
patch/kernel/archive/qcs6490-6.18/0046-drm-panel-Add-support-for-Orise-Tech-OTA7290B-DSI-Pa.patch (1)
201-226: Inconsistent physical dimensions between display mode and panel descriptor.The
drm_display_modestruct defineswidth_mm = 135andheight_mm = 217(lines 212-213), while thepanel_descuseswidth_mm = 74andheight_mm = 131(lines 220-221). Theget_modesfunction uses thepanel_descvalues for the connector's display info.For a "10FHD" (10-inch Full HD) panel, the dimensions 135×217mm are more plausible. The current 74×131mm values suggest a ~6-inch display instead.
Suggested fix if 135×217mm is correct
static const struct panel_desc radxa_display_10fhd_ad003_desc = { .display_mode = &radxa_display_10fhd_ad003_1200x1920, - .width_mm = 74, - .height_mm = 131, + .width_mm = 135, + .height_mm = 217, .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_NO_EOT_PACKET,patch/kernel/archive/qcs6490-6.18/0047-arm64-dts-qcom-sc7280-Add-cci-aliases.patch (1)
19-22: Copy-paste error:i2c19alias points to wrong CCI I2C controller.Line 22 assigns
i2c19to&cci1_i2c0, but based on the naming pattern, it should reference&cci1_i2c1:
Alias Expected Target i2c16 cci0_i2c0 ✓ i2c17 cci0_i2c1 ✓ i2c18 cci1_i2c0 ✓ i2c19 cci1_i2c1 ✗ Currently, both
i2c18andi2c19alias to the samecci1_i2c0controller, which would causecci1_i2c1to be inaccessible via the alias mechanism.🐛 Proposed fix
i2c16 = &cci0_i2c0; i2c17 = &cci0_i2c1; i2c18 = &cci1_i2c0; - i2c19 = &cci1_i2c0; + i2c19 = &cci1_i2c1;
🤖 Fix all issues with AI agents
In
`@patch/kernel/archive/qcs6490-6.18/0040-crypto-qce-Add-runtime-PM-and-interconnect-bandwidth.patch`:
- Around line 104-127: Revert the replacement of optional clock handling with
pm_clk_add for "iface" and "bus": use devm_clk_get_optional (or
devm_clk_get_optional_enabled if you need it enabled) to fetch qce->iface and
qce->bus, check IS_ERR and return PTR_ERR on error, then only call
pm_clk_add_clk (or pm_clk_add_clk equivalent) when the returned clk is non-NULL
and handle its return value; update the probe code paths around qce->iface and
qce->bus to match this pattern so platforms missing these optional clocks
continue to probe successfully.
In
`@patch/kernel/archive/qcs6490-6.18/0066-spi-geni-qcom-rework-setup_fifo_params.patch`:
- Around line 60-77: The code incorrectly tests mode_changed against
register-offset constants SE_SPI_CPHA/SE_SPI_CPOL instead of the SPI mode bit
masks, so CPHA/CPOL updates never trigger; change the two conditionals that read
"if (mode_changed & SE_SPI_CPHA)" and "if (mode_changed & SE_SPI_CPOL)" to test
against the SPI mode masks "SPI_CPHA" and "SPI_CPOL" respectively (keep the
write logic using SE_SPI_CPHA/SE_SPI_CPOL offsets and the existing writes that
use spi_slv->mode to decide CPHA/CPOL). Ensure you reference the variables
mode_changed, spi_slv->mode and the constants SPI_CPHA/SPI_CPOL in the patch.
In
`@patch/kernel/archive/qcs6490-6.18/0071-r8169-Remove-the-check-of-a-register-to-enable-ASPM-.patch`:
- Around line 23-26: The patch removes the r8168_mac_ocp_read(0xc0b2) register
gate and enables ASPM for all devices with tp->mac_version >=
RTL_GIGA_MAC_VER_46; restore vendor scoping by either reintroducing the original
register check (r8168_mac_ocp_read(tp, 0xc0b2) & 0xf) or add a device-specific
quirk/filter that checks the PCI subsystem/device ID or a device-tree compatible
string (e.g., Radxa Dragon Q6A) before returning true so only the intended
hardware (not all RTL_GIGA_MAC_VER_46 devices) enables ASPM.
In
`@patch/kernel/archive/qcs6490-6.18/0072-media-venus-Workaround-10-bit-decoding-and-performan.patch`:
- Around line 41-46: The commented-out QC10C validation in find_format and
find_format_by_index (the checks comparing fmt[i].pixfmt == V4L2_PIX_FMT_QC10C
and inst->bit_depth == VIDC_BITDEPTH_10) must be resolved: either remove the
commented lines entirely and add a brief commit message explaining the
workaround rationale, or reintroduce active validation with explicit logic
(e.g., skip the check until inst->bit_depth is initialized or return NULL when
bit depth != 10) and add an in-code comment explaining why the bypass is safe;
update the functions find_format and find_format_by_index accordingly and ensure
any deferred-validation path is clearly documented in the code comment
referencing inst->bit_depth and V4L2_PIX_FMT_QC10C.
- Around line 17-30: Define or replace the undefined VENUS_MAX_FPS and reconcile
the decide_core fallback behavior: either add a local definition for
VENUS_MAX_FPS (e.g. match upstream value like 240) or replace uses of
VENUS_MAX_FPS with an explicit literal, and in decide_core() fix the log and
behavior to be consistent by removing commented-out lines, ensuring the dev_warn
message matches the actual actions (if enabling power save via
power_save_mode_enable(inst, true) then log that we're falling back to
power-save/minimal operation; if instead you intend to force max performance
clear the power-save call and set
inst->clk_data.core_id/cu.video_core_enable_mask accordingly) so that
dev_warn(core->dev, ...), inst->clk_data.core_id, cu.video_core_enable_mask and
power_save_mode_enable() all reflect the same intent.
In
`@patch/kernel/archive/qcs6490-6.18/0084-soc-qcom-ice-Add-HWKM-v1-support-for-wrapped-keys.patch`:
- Around line 177-185: The exported functions qcom_ice_generate_key,
qcom_ice_prepare_key, and qcom_ice_import_key now use
QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version) but do not guard against
unsupported HWKM on callers; add a runtime check at the top of each function
(e.g. if (!ice->use_hwkm || ice->hwkm_version == 0) return -ENODEV or
-EOPNOTSUPP) to fail fast for SoCs without HWKM support, or alternatively
document in each exported function header that callers must verify ice->use_hwkm
is true before calling; update all three functions (qcom_ice_generate_key,
qcom_ice_prepare_key, qcom_ice_import_key) to use this guard.
In
`@patch/kernel/archive/qcs6490-6.18/0091-drm-panel-jd9365da-h3-add-enable-callback.patch`:
- Around line 19-53: The init() call was moved into jadard_enable(), breaking
the required sequence; revert that by removing the jadard->desc->init(jadard)
call from jadard_enable() (or delete jadard_enable entirely) and ensure
jadard->desc->init(jadard) is called in jadard_prepare() immediately after the
hardware reset sequence (the gpiod_set_value(jadard->reset, 0); msleep(130);
lines). Also remove the .enable = jadard_enable entry from the jadard_funcs
table (or leave .enable NULL) so init() is only invoked from jadard_prepare()
following reset.
In
`@patch/kernel/archive/qcs6490-6.18/0098-arm64-dts-meson-radxa-zero2-enable-the-npu-node.patch`:
- Around line 1-12: The patch 0098 (arm64 dts: meson: radxa-zero2: enable the
npu node) is misplaced in the qcs6490-6.18 archive and duplicates an existing
patch in the meson64-6.18 archive; remove this duplicate from the qcs6490-6.18
series (delete or exclude patch 0098 from that directory) and ensure the single
authoritative patch remains in the meson64-6.18 series (the file named
board-radxa-zero2-arm64-dts-amlogic-Enable-the-npu-node-for-Radxa-Zero2.patch),
update the series/index so it references only the meson64 location, and
check/remove other similarly misplaced Radxa Zero 2 patches (0095, 0097, 0113,
0117, 0118, 0120, 0121) from the qcs6490-6.18 directory to avoid duplicates.
In
`@patch/kernel/archive/qcs6490-6.18/0099-arm64-dts-meson-g12a-modify-cpu-opp-voltage-value.patch`:
- Around line 1-13: The patch modifies
arch/arm64/boot/dts/amlogic/meson-g12a.dtsi (CPU OPP voltage values) but is
incorrectly placed under the qcs6490-6.18 patch series; move this patch file
into the meson64-6.18 directory so Amlogic Meson SoC changes are grouped
correctly. Update the patch metadata (subject/series location) if needed so the
patch applies in the meson64-6.18 queue and remove it from qcs6490-6.18; ensure
the patch filename and the referenced target (meson-g12a.dtsi) remain unchanged
and that any series index/order is adjusted to match other meson64-6.18 patches.
In
`@patch/kernel/archive/qcs6490-6.18/0104-drm-msm-dpu-Check-mode-against-PINGPONG-or-DSC-max-w.patch`:
- Around line 32-55: The helper msm_display_get_max_pingpong_width can return 0
if any pingpong->max_linewidth is zero, causing all modes to be rejected; update
it to ignore zero max_linewidth entries and fall back to a sane default:
initialize max_pingpong_width from dpu_kms->caps->max_mixer_width (or use it if
no non-zero pingpong widths are found), and when iterating over
dpu_kms->catalog->pingpong[i] only consider pingpong->max_linewidth values > 0
when computing the minimum; ensure the function returns the fallback
caps->max_mixer_width if all pingpong entries are zero.
In
`@patch/kernel/archive/qcs6490-6.18/0109-arm64-dts-meson-radxa-zero-add-USB-Type-C-support-vi.patch`:
- Around line 79-89: The &dwc3 node uses an incorrect container name "ports";
change it to a single "port" node so the endpoint child nodes (usb_dwc2_out:
endpoint@0 and usb_dwc3_out: endpoint@1) attach correctly to &dwc3 and follow
the OF-graph binding; update the block around &dwc3 to replace "ports { ... }"
with "port { ... }" while keeping the remote-endpoint references to &ucsi0_hs_in
and &ucsi0_ss_in unchanged.
In
`@patch/kernel/archive/qcs6490-6.18/0111-drm-edid-add-device-tree-quirk-flags-supports.patch`:
- Around line 38-56: The connector_get_quirks() function uses
EDID_QUIRK_PREFER_LARGE_60 and EDID_QUIRK_PREFER_LARGE_75 but those enum members
are missing; add EDID_QUIRK_PREFER_LARGE_60 and EDID_QUIRK_PREFER_LARGE_75 to
the enum drm_edid_internal_quirk so BIT(EDID_QUIRK_PREFER_LARGE_60) and
BIT(EDID_QUIRK_PREFER_LARGE_75) compile correctly, keeping them alongside
CONNECTOR_QUIRK_PREFER_FHD and preserving the intended bit positions.
In
`@patch/kernel/archive/qcs6490-6.18/0119-arm64-dts-amlogic-meson-g12b-add-missing-PWM-pinctrl.patch`:
- Around line 1-16: The patch
"0119-arm64-dts-amlogic-meson-g12b-add-missing-PWM-pinctrl.patch" was committed
into the Qualcomm archive (qcs6490-6.18) but belongs in the Amlogic archive;
move that patch file from the qcs6490-6.18 patch archive into the meson64-6.18
patch archive, preserve the original commit/author/Signed-off-by metadata, and
update any patch series/index (or quilt/mbox list) references so the patch is
applied with the other meson-g12b/meson64 patches.
🟡 Minor comments (8)
patch/kernel/archive/qcs6490-6.18/0061-mtd-devices-Add-Qualcomm-SCM-storage-driver.patch-276-276 (1)
276-276: Potential integer overflow in size calculation.If
total_blocksandblock_sizeare both 32-bit types, this multiplication will be performed in 32-bit arithmetic before assignment to the 64-bitmtd.size, potentially causing overflow for large flash devices.🔧 Suggested fix
- host->mtd.size = host->info.total_blocks * host->info.block_size; + host->mtd.size = (u64)host->info.total_blocks * host->info.block_size;patch/kernel/archive/qcs6490-6.18/0061-mtd-devices-Add-Qualcomm-SCM-storage-driver.patch-73-77 (1)
73-77: Remove unused#include <linux/dma-mapping.h>header.The
dma-mapping.hheader is included but no DMA mapping functions are used in this driver. The SCM layer (qcom_scm_storage_send_cmd) internally allocates and manages its own TrustZone-protected memory pool viaqcom_tzmem_alloc, copying the caller's buffer as needed. The bounce buffer allocated at line 252 withdevm_kzalloc(dev, host->buffer_size, GFP_KERNEL)is correct and does not require DMA-coherent memory. Remove the unused include.patch/kernel/archive/qcs6490-6.18/0052-arm64-dts-qcom-sc7280-Describe-the-MBI-functionality.patch-38-45 (1)
38-45: Add#msi-cells = <1>;for GICv3 MSI controller compliance.When marking the GIC node as an MSI controller via
msi-controller, the kernel'sarm,gic-v3.yamlbinding requires the#msi-cellsproperty. This property defines the cell size for MSI device IDs and must be present to satisfydtbs_checkvalidation.Required property addition
mbi-alias = <0x0 0x17a10000>; mbi-ranges = <832 128>; msi-controller; + `#msi-cells` = <1>; `#interrupt-cells` = <3>;patch/kernel/archive/qcs6490-6.18/0107-usb-typec-tcpm-support-skipping-VSafe0V-check.patch-41-52 (1)
41-52: Add binding documentation for thetcpm,skip-vsafe0v-checkproperty.Patch 0107 introduces driver code that consumes the
tcpm,skip-vsafe0v-checkdevice property, and patches 0109 and 0113 use it in device trees. This property should be documented in the TCPM device tree binding YAML file to ensure DT schema validation and maintain consistency with kernel binding definitions.patch/kernel/archive/qcs6490-6.18/0001-dt-bindings-phy-qcom-sc8280xp-qmp-usb43dp-phy-Docume.patch-64-90 (1)
64-90: Fix data-lanes constraints for DisplayPort endpoint to match oneOf enumeration.The endpoint@0
data-lanesproperty declaresminItems: 2but includes oneOf branches for 1-lane DisplayPort configurations (single-element arrays). This constraint mismatch will cause dt-schema validation failures for valid 1-lane configurations. ChangeminItemsto 1 for the DisplayPort endpoint, or remove the 1-lane options if they are not actually supported.patch/kernel/archive/qcs6490-6.18/0011-remoteproc-core-Allow-restarting-detached-remoteproc.patch-214-214 (1)
214-214: Typo in comment: "avilable" → "available"- /* Check early if we have firmware avilable if needed */ + /* Check early if we have firmware available if needed */patch/kernel/archive/qcs6490-6.18/0011-remoteproc-core-Allow-restarting-detached-remoteproc.patch-343-350 (1)
343-350: Documentation comment uses wrong enum name.The comment references
RPROC_FEAT_REBOOT_IF_FW_AVAILABLEbut the actual enum value defined at line 355 isRPROC_AUTO_BOOT_RESTART_IF_FW_AVAILABLE. This mismatch could cause confusion.Proposed fix
- * `@RPROC_FEAT_REBOOT_IF_FW_AVAILABLE`: The remoteproc will be restarted if + * `@RPROC_AUTO_BOOT_RESTART_IF_FW_AVAILABLE`: The remoteproc will be restarted ifpatch/kernel/archive/qcs6490-6.18/0027-hack-soc-qcom-pmic_glink-Avoid-platform-rpmsg-probe-.patch-35-47 (1)
35-47: Add guard or document single-instance requirement for rpmsg driver registration.The patch registers the rpmsg driver in
pmic_glink_probe(), which will fail with-EBUSY/-EEXISTif multiple PMIC GLINK platform devices probe (sinceregister_rpmsg_driver()cannot be called twice for the same driver name). Either add a flag to ensure registration happens once, or explicitly document that the DT/SoC guarantees a single instance and will not change.
🧹 Nitpick comments (14)
patch/kernel/archive/qcs6490-6.18/0035-scripts-Add-PKGBUILD-for-testing-on-Arch-Linux-ARM.patch (1)
120-124: Inconsistent indentation in dtbs_install block.Line 123 uses spaces while line 122 uses a tab. While this won't break functionality, it's inconsistent within the same code block.
🔧 Suggested fix for consistency
if [ -d "${srctree}/arch/${SRCARCH}/boot/dts" ]; then echo "Installing dtbs..." ${MAKE} INSTALL_DTBS_PATH="${modulesdir}/dtb" dtbs_install - ${MAKE} INSTALL_DTBS_PATH="${pkgdir}/boot/dtb" dtbs_install + ${MAKE} INSTALL_DTBS_PATH="${pkgdir}/boot/dtb" dtbs_install fipatch/kernel/archive/qcs6490-6.18/0060-firmware-qcom-scm-Add-SCM-storage-interface-support.patch (1)
121-139: Payload copy-back occurs before result validation.The payload is copied back to the caller at lines 121-122 before checking
scm_res.result[0]. On failed storage operations (e.g.,ACCESS_DENIED,STORAGE_ERROR), this could copy potentially invalid or partial data back to the caller.Consider moving the copy-back inside the
STORAGE_RESULT_SUCCESScase:♻️ Proposed fix
if (ret) return ret; - if (payload) - memcpy(payload, payload_buf, size); - switch (scm_res.result[0]) { case STORAGE_RESULT_SUCCESS: + if (payload) + memcpy(payload, payload_buf, size); return 0; case STORAGE_RESULT_NO_MEMORY:patch/kernel/archive/qcs6490-6.18/0068-WIP-change-reserved-mem-layout-for-WP.patch (1)
1-6: WIP patch included in the series — consider finalizing before merge.The patch subject indicates "WIP" (Work-In-Progress) status. Given the PR description confirms successful boot testing with the new BIOS snapshot (
dragon-q6a_flat_build_wp_260120) and verified functionality (desktop, Wi-Fi, media playback, audio), this appears production-ready despite the label.Consider removing the "WIP:" prefix from the commit subject if no further changes are expected, to avoid confusion in the patch history.
patch/kernel/archive/qcs6490-6.18/0099-arm64-dts-meson-g12a-modify-cpu-opp-voltage-value.patch (1)
6-8: Reference points to an old vendor kernel from 2020.The reference
amlogic-4.9-20is a vendor kernel based on Linux 4.9. Consider verifying these voltage values against more recent upstream sources or confirming this aligns with current mainline recommendations for Meson G12A.patch/kernel/archive/qcs6490-6.18/0023-wip-remoteproc-qcom_q6v5_pas-Avoid-using-broken-rese.patch (1)
83-88: Early exit returns success despite non-functional driver.When reset is broken and remoteproc isn't running, the driver exits with
ret = 0to avoid blockingsync_state(). While the comment explains the rationale, returning success for a non-functional probe could mask issues in system integration. Consider whether a distinct return code or logging at a higher severity would help operators identify misconfigured systems.patch/kernel/archive/qcs6490-6.18/0040-crypto-qce-Add-runtime-PM-and-interconnect-bandwidth.patch (1)
191-209: Unnecessary initialization on line 195.The variable
retis initialized to 0 but immediately overwritten byicc_enable()on line 196. This is a minor style nit—can be simplified toint ret;.🔧 Optional cleanup
static int __maybe_unused qce_runtime_resume(struct device *dev) { struct qce_device *qce = dev_get_drvdata(dev); - int ret = 0; + int ret; ret = icc_enable(qce->mem_path);patch/kernel/archive/qcs6490-6.18/0094-arm64-dts-amlogic-enable-device-tree-overlay-support.patch (1)
22-27: Consider using+=instead of:=for DTC_FLAGS.Using
:=will override any DTC_FLAGS that might be set earlier in the build chain. While this typically works fine, using+=is a safer pattern that preserves any global flags:-DTC_FLAGS := -@ +DTC_FLAGS += -@This aligns with patterns used in other arch DT Makefiles that append to DTC_FLAGS rather than replacing them.
patch/kernel/archive/qcs6490-6.18/0088-drm-msm-dp-Filter-out-certain-modes-to-avoid-instabi.patch (1)
31-33: Consider adding a comment explaining the hardware limitation.The magic numbers (3440×1440, 30Hz) would benefit from a brief inline comment explaining why this specific threshold was chosen. This helps future maintainers understand the hardware constraint:
+ /* QCS6490 DP has stability issues above 3440x1440 at >30Hz refresh rates */ if ((mode->hdisplay * mode->vdisplay > 3440 * 1440) && vrefresh > 30) return MODE_BAD;The logic itself is correct—modes at exactly 3440×1440 or at 30Hz and below remain valid.
patch/kernel/archive/qcs6490-6.18/0121-arm64-dts-meson-radxa-zero-series-enhance-WiFi-SDIO-.patch (1)
29-42: Zero2 lackssd-uhs-sdr50while Zero has it—consider adding for consistency.The patch shows Zero (g12a) already has
sd-uhs-sdr50defined before addingsd-uhs-sdr104, but Zero2 (g12b) does not. While the patch successfully enables SDR104 for both devices and testing confirms WiFi works, addingsd-uhs-sdr50to Zero2 would align the configurations and reflect the full UHS mode hierarchy.patch/kernel/archive/qcs6490-6.18/0000.patching_config.yaml (1)
3-8: Consider removing the unused “info stuff” metadata block to reduce churn.
The comment already states it is not used by patching scripts, so trimming it keeps future rebases cleaner.Based on learnings, remove unused metadata rather than updating it.♻️ Proposed cleanup
- # Just some info stuff; not used by the patching scripts - name: qcs6490-6.18 - kind: kernel - type: mainline # or: vendor - branch: linux-6.18.y - last-known-good-tag: v6.18.2patch/kernel/archive/qcs6490-6.18/0108-usb-typec-fusb302-workaround-for-fixed-VBUS-hardware.patch (1)
53-58: DT property binding may need documentation.The
fcs,force-vbus-toggleproperty follows the correct vendor prefix convention. Consider whether a dt-bindings documentation update is needed for upstream acceptance, though this may be deferred for Armbian's vendor patch context.config/boards/radxa-dragon-q6a.conf (1)
33-52: Add checksum verification and error handling for external package download.The function downloads a
.debfrom an external URL without verifying its integrity. This introduces potential security and reliability issues:
- No checksum verification: A compromised or corrupted package could be installed.
- No download error handling: If
wgetfails,dpkg -iwill fail on a missing/incomplete file.- Hardcoded version: May become stale as Radxa releases updates.
♻️ Suggested improvements
function post_family_tweaks__radxa-dragon-q6a_install_alsa_ucm_conf() { # Only install for kernel 6.18.x if [[ "${KERNEL_MAJOR_MINOR}" == "6.18" ]]; then display_alert "Installing alsa-ucm-conf" "${BOARD}: Radxa ALSA UCM configuration" "info" declare alsa_ucm_url="https://github.com/radxa-pkg/alsa-ucm-conf/releases/download/1.2.14-1radxa2/alsa-ucm-conf_1.2.14-1radxa2_all.deb" declare alsa_ucm_deb="/tmp/alsa-ucm-conf_1.2.14-1radxa2_all.deb" + declare alsa_ucm_sha256="<insert_expected_sha256_here>" # Download the package - run_host_command_logged wget -O "${SDCARD}${alsa_ucm_deb}" "${alsa_ucm_url}" + if ! run_host_command_logged wget -q -O "${SDCARD}${alsa_ucm_deb}" "${alsa_ucm_url}"; then + display_alert "Failed to download alsa-ucm-conf" "${alsa_ucm_url}" "warn" + return 0 + fi + + # Verify checksum + if ! echo "${alsa_ucm_sha256} ${SDCARD}${alsa_ucm_deb}" | sha256sum -c - > /dev/null 2>&1; then + display_alert "Checksum verification failed" "alsa-ucm-conf" "warn" + rm -f "${SDCARD}${alsa_ucm_deb}" + return 0 + fi # Install in chroot chroot_sdcard dpkg -i "${alsa_ucm_deb}"patch/kernel/archive/qcs6490-6.18/0070-arm64-configs-Import-qcom_module_defconfig-from-drag.patch (1)
2221-2237: Consider moving debug-heavy options to a debug-only defconfig.This defconfig enables DEBUG_KERNEL, DYNAMIC_DEBUG, MAGIC_SYSRQ, KFENCE, and related tracing options. If this defconfig feeds production images, these add overhead and expand the debug surface; if it’s dev-only, all good—otherwise consider splitting or disabling them.
patch/kernel/archive/qcs6490-6.18/0029-rpmsg-core-Make-it-easier-to-manually-create-endpoin.patch (1)
65-88: Consider a defensive guard against double-open.If a driver calls
rpmsg_dev_open_ept()twice, the previous endpoint is leaked andrpdev->eptis overwritten. AWARN_ON/early return makes misuse explicit.♻️ Suggested guard
struct rpmsg_endpoint *rpmsg_dev_open_ept(struct rpmsg_device *rpdev, rpmsg_rx_cb_t cb, void *priv) { struct rpmsg_driver *rpdrv = to_rpmsg_driver(rpdev->dev.driver); + if (WARN_ON(rpdev->ept)) + return NULL; struct rpmsg_channel_info chinfo = { .src = rpdev->src, .dst = RPMSG_ADDR_ANY, };
.../kernel/archive/qcs6490-6.18/0040-crypto-qce-Add-runtime-PM-and-interconnect-bandwidth.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/qcs6490-6.18/0066-spi-geni-qcom-rework-setup_fifo_params.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0071-r8169-Remove-the-check-of-a-register-to-enable-ASPM-.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0072-media-venus-Workaround-10-bit-decoding-and-performan.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0072-media-venus-Workaround-10-bit-decoding-and-performan.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/qcs6490-6.18/0099-arm64-dts-meson-g12a-modify-cpu-opp-voltage-value.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0104-drm-msm-dpu-Check-mode-against-PINGPONG-or-DSC-max-w.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0109-arm64-dts-meson-radxa-zero-add-USB-Type-C-support-vi.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/qcs6490-6.18/0111-drm-edid-add-device-tree-quirk-flags-supports.patch
Outdated
Show resolved
Hide resolved
.../kernel/archive/qcs6490-6.18/0119-arm64-dts-amlogic-meson-g12b-add-missing-PWM-pinctrl.patch
Outdated
Show resolved
Hide resolved
amazingfate
left a comment
There was a problem hiding this comment.
Please work based on the current patches instead of overwriting all of them, it makes review hard. And radxa's kernel tree has some unnecessary patches, I don't want them included.
.../kernel/archive/qcs6490-6.18/0003-arm64-dts-qcom-x1e78100-lenovo-thinkpad-t14s-add-HDM.patch
Outdated
Show resolved
Hide resolved
|
Drive-by comment: I would remove all patches that have "meson" or "amlogic" in their name, as those are completely irrelevant here. |
f6ea9aa to
729ff77
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/boards/radxa-dragon-q6a.conf (1)
33-52: Consider adding error handling for the download step.The function downloads a
.debpackage from GitHub without verifying that the download succeeded. Ifwgetfails (network issue, URL change, rate limiting),dpkg -iwill operate on a missing or corrupted file, causing a confusing build failure.♻️ Proposed fix with error handling
function post_family_tweaks__radxa-dragon-q6a_install_alsa_ucm_conf() { # Only install for kernel 6.18.x if [[ "${KERNEL_MAJOR_MINOR}" == "6.18" ]]; then display_alert "Installing alsa-ucm-conf" "${BOARD}: Radxa ALSA UCM configuration" "info" declare alsa_ucm_url="https://github.com/radxa-pkg/alsa-ucm-conf/releases/download/1.2.14-1radxa2/alsa-ucm-conf_1.2.14-1radxa2_all.deb" declare alsa_ucm_deb="/tmp/alsa-ucm-conf_1.2.14-1radxa2_all.deb" # Download the package - run_host_command_logged wget -O "${SDCARD}${alsa_ucm_deb}" "${alsa_ucm_url}" + if ! run_host_command_logged wget -O "${SDCARD}${alsa_ucm_deb}" "${alsa_ucm_url}"; then + display_alert "Failed to download alsa-ucm-conf" "${BOARD}: check network or URL" "warn" + return 1 + fi # Install in chroot chroot_sdcard dpkg -i "${alsa_ucm_deb}" # Clean up run_host_command_logged rm -f "${SDCARD}${alsa_ucm_deb}" else display_alert "Skipping alsa-ucm-conf installation" "Kernel version ${KERNEL_MAJOR_MINOR} != 6.18" "info" fi }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/radxa-dragon-q6a.conf` around lines 33 - 52, The download step in post_family_tweaks__radxa-dragon-q6a_install_alsa_ucm_conf() does not check that wget succeeded before running chroot_sdcard dpkg -i; modify the sequence to check the exit status and/or verify the file exists and is non-empty at "${SDCARD}${alsa_ucm_deb}" after run_host_command_logged wget, and if the download failed, call display_alert or processLogger/error and skip the dpkg installation and cleanup steps (or return non-zero). Ensure you only run chroot_sdcard dpkg -i "${alsa_ucm_deb}" and run_host_command_logged rm -f "${SDCARD}${alsa_ucm_deb}" when the download verification passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/boards/radxa-dragon-q6a.conf`:
- Around line 33-52: The download step in
post_family_tweaks__radxa-dragon-q6a_install_alsa_ucm_conf() does not check that
wget succeeded before running chroot_sdcard dpkg -i; modify the sequence to
check the exit status and/or verify the file exists and is non-empty at
"${SDCARD}${alsa_ucm_deb}" after run_host_command_logged wget, and if the
download failed, call display_alert or processLogger/error and skip the dpkg
installation and cleanup steps (or return non-zero). Ensure you only run
chroot_sdcard dpkg -i "${alsa_ucm_deb}" and run_host_command_logged rm -f
"${SDCARD}${alsa_ucm_deb}" when the download verification passes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (37)
config/boards/radxa-dragon-q6a.confconfig/kernel/linux-qcs6490-current.configconfig/kernel/linux-qcs6490-edge.configconfig/sources/families/qcs6490.confpatch/kernel/archive/qcs6490-6.18/0004-arm64-dts-qcom-qcs6490-audioreach-Modify-LPASS-macro.patchpatch/kernel/archive/qcs6490-6.18/0010-phy-qcom-qmp-combo-temporarily-fix-the-orientation-t.patchpatch/kernel/archive/qcs6490-6.18/0016-firmware-psci-Read-and-use-vendor-reset-types.patchpatch/kernel/archive/qcs6490-6.18/0018-drm-birdge-simple-bridge-Add-support-for-radxa-ra620.patchpatch/kernel/archive/qcs6490-6.18/0019-scsi-ufs-qcom-temporarily-force-to-rate-A.patchpatch/kernel/archive/qcs6490-6.18/0020-phy-qcom-qmp-ufs-Fix-HS-G4-PHY-init-table-for-sc7280.patchpatch/kernel/archive/qcs6490-6.18/0021-arm64-dts-qcom-sc7280-add-OPP-table-support-to-PCIe-.patchpatch/kernel/archive/qcs6490-6.18/0022-drm-msm-dpu-Add-missing-caps-and-blocks-to-sc7280-ca.patchpatch/kernel/archive/qcs6490-6.18/0023-remoteproc-core-Allow-auto-retry-of-rproc_start.patchpatch/kernel/archive/qcs6490-6.18/0024-scripts-Add-PKGBUILD-for-testing-on-Arch-Linux-ARM.patchpatch/kernel/archive/qcs6490-6.18/0028-configs-Add-qcom_module_defconfig.patchpatch/kernel/archive/qcs6490-6.18/0029-spi-spi-qcom-qspi-Add-support-for-polling.patchpatch/kernel/archive/qcs6490-6.18/0030-drm-panel-Add-support-for-Orise-Tech-OTA7290B-DSI-Pa.patchpatch/kernel/archive/qcs6490-6.18/0031-arm64-dts-qcom-sc7280-Add-cci-aliases.patchpatch/kernel/archive/qcs6490-6.18/0032-arm64-dts-qcom-Add-initial-support-for-Radxa-Dragon-.patchpatch/kernel/archive/qcs6490-6.18/0033-arm64-dts-qcom-radxa-dragon-q6a-Add-user-led.patchpatch/kernel/archive/qcs6490-6.18/0034-arm64-dts-qcom-Add-qcs6490-dragon-q6a-radxa-display-.patchpatch/kernel/archive/qcs6490-6.18/0035-soc-qcom-geni-se-Add-MODULE_FIRMWARE-macro.patchpatch/kernel/archive/qcs6490-6.18/0036-arm64-configs-Update-qcom_module_defconfig.patchpatch/kernel/archive/qcs6490-6.18/0037-soc-qcom-rpmh-Add-support-to-read-back-resource-sett.patchpatch/kernel/archive/qcs6490-6.18/0038-regulator-qcom-rpmh-Add-support-to-read-regulator-se.patchpatch/kernel/archive/qcs6490-6.18/0039-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Optimize-reg.patchpatch/kernel/archive/qcs6490-6.18/0040-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Move-LED-nod.patchpatch/kernel/archive/qcs6490-6.18/0041-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Fix-pcie0_cl.patchpatch/kernel/archive/qcs6490-6.18/0042-scsi-ufs-qcom-Add-support-for-UFS-module-detection.patchpatch/kernel/archive/qcs6490-6.18/0043-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Add-UFS-modu.patchpatch/kernel/archive/qcs6490-6.18/0044-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Fix-overlays.patchpatch/kernel/archive/qcs6490-6.18/0045-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Add-an-overl.patchpatch/kernel/archive/qcs6490-6.18/0046-arm64-dts-qcom-qcs6490-dragon-q6a-add-gpio-label.patchpatch/kernel/archive/qcs6490-6.18/0047-drivers-gpu-drm-fix-4K-30-fps.patchpatch/kernel/archive/qcs6490-6.18/0048-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Enable-eMMC.patchpatch/kernel/archive/qcs6490-6.18/0049-drm-display-add-hw_params-callback-function-to-drm_c.patchpatch/kernel/archive/qcs6490-6.18/0050-6.18-VP9-Fix.patch
💤 Files with no reviewable changes (33)
- patch/kernel/archive/qcs6490-6.18/0042-scsi-ufs-qcom-Add-support-for-UFS-module-detection.patch
- patch/kernel/archive/qcs6490-6.18/0023-remoteproc-core-Allow-auto-retry-of-rproc_start.patch
- patch/kernel/archive/qcs6490-6.18/0036-arm64-configs-Update-qcom_module_defconfig.patch
- patch/kernel/archive/qcs6490-6.18/0020-phy-qcom-qmp-ufs-Fix-HS-G4-PHY-init-table-for-sc7280.patch
- patch/kernel/archive/qcs6490-6.18/0030-drm-panel-Add-support-for-Orise-Tech-OTA7290B-DSI-Pa.patch
- patch/kernel/archive/qcs6490-6.18/0049-drm-display-add-hw_params-callback-function-to-drm_c.patch
- patch/kernel/archive/qcs6490-6.18/0028-configs-Add-qcom_module_defconfig.patch
- patch/kernel/archive/qcs6490-6.18/0044-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Fix-overlays.patch
- patch/kernel/archive/qcs6490-6.18/0032-arm64-dts-qcom-Add-initial-support-for-Radxa-Dragon-.patch
- patch/kernel/archive/qcs6490-6.18/0035-soc-qcom-geni-se-Add-MODULE_FIRMWARE-macro.patch
- patch/kernel/archive/qcs6490-6.18/0048-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Enable-eMMC.patch
- patch/kernel/archive/qcs6490-6.18/0031-arm64-dts-qcom-sc7280-Add-cci-aliases.patch
- patch/kernel/archive/qcs6490-6.18/0046-arm64-dts-qcom-qcs6490-dragon-q6a-add-gpio-label.patch
- patch/kernel/archive/qcs6490-6.18/0047-drivers-gpu-drm-fix-4K-30-fps.patch
- patch/kernel/archive/qcs6490-6.18/0033-arm64-dts-qcom-radxa-dragon-q6a-Add-user-led.patch
- patch/kernel/archive/qcs6490-6.18/0019-scsi-ufs-qcom-temporarily-force-to-rate-A.patch
- patch/kernel/archive/qcs6490-6.18/0016-firmware-psci-Read-and-use-vendor-reset-types.patch
- patch/kernel/archive/qcs6490-6.18/0038-regulator-qcom-rpmh-Add-support-to-read-regulator-se.patch
- patch/kernel/archive/qcs6490-6.18/0034-arm64-dts-qcom-Add-qcs6490-dragon-q6a-radxa-display-.patch
- patch/kernel/archive/qcs6490-6.18/0039-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Optimize-reg.patch
- patch/kernel/archive/qcs6490-6.18/0045-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Add-an-overl.patch
- patch/kernel/archive/qcs6490-6.18/0037-soc-qcom-rpmh-Add-support-to-read-back-resource-sett.patch
- patch/kernel/archive/qcs6490-6.18/0021-arm64-dts-qcom-sc7280-add-OPP-table-support-to-PCIe-.patch
- patch/kernel/archive/qcs6490-6.18/0029-spi-spi-qcom-qspi-Add-support-for-polling.patch
- patch/kernel/archive/qcs6490-6.18/0040-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Move-LED-nod.patch
- patch/kernel/archive/qcs6490-6.18/0004-arm64-dts-qcom-qcs6490-audioreach-Modify-LPASS-macro.patch
- patch/kernel/archive/qcs6490-6.18/0010-phy-qcom-qmp-combo-temporarily-fix-the-orientation-t.patch
- patch/kernel/archive/qcs6490-6.18/0018-drm-birdge-simple-bridge-Add-support-for-radxa-ra620.patch
- patch/kernel/archive/qcs6490-6.18/0041-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Fix-pcie0_cl.patch
- patch/kernel/archive/qcs6490-6.18/0022-drm-msm-dpu-Add-missing-caps-and-blocks-to-sc7280-ca.patch
- patch/kernel/archive/qcs6490-6.18/0043-arm64-dts-qcom-qcs6490-radxa-dragon-q6a-Add-UFS-modu.patch
- patch/kernel/archive/qcs6490-6.18/0050-6.18-VP9-Fix.patch
- patch/kernel/archive/qcs6490-6.18/0024-scripts-Add-PKGBUILD-for-testing-on-Arch-Linux-ARM.patch
|
For sake of maintainability removed us from handling patches manually & switched 6.18.2 to current. |
|
@igorpecovnik ready now. |
|
✅ This PR has been reviewed and approved — all set for merge! |
Description
git format-patchto get latest kernel patches from Radxa on top of 6.18.2Reasoning
Lots of changes since initial support/bios for Radxa-Dragon-Q6A. This makes it compatible with the latest updates
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Hardware Support