Added config improvements and fixes to ayn odin2#9316
Added config improvements and fixes to ayn odin2#9316Squishy123 wants to merge 26 commits intoarmbian:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two Ayn Odin2 GRUB board configs and first-run presets; BSP audio/loader configs; an SM8550 kernel defconfig and family mapping change; and a large sm8550-6.13 kernel patch series introducing the Qualcomm Iris video accelerator driver and associated DT, drivers, power, V4L2/VB2, firmware, and helper changes, plus miscellaneous drivers and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Userspace App
participant V4L2 as V4L2/VB2
participant Iris as Iris Driver (core)
participant HFI as HFI Queues / Host‑FW IPC
participant VPU as VPU Firmware
participant PM as PM/Resources
App->>V4L2: open + ioctls (REQBUF / QBUF / STREAMON)
V4L2->>Iris: forward vb2 ops / ioctls
Iris->>PM: pm_runtime_resume / prepare power, clocks, ICC
Iris->>HFI: enqueue session_start / config / buffer commands
HFI->>VPU: deliver commands via shared queues
VPU-->>HFI: response / interrupt
HFI->>Iris: ISR invokes response handlers
Iris->>V4L2: complete vb2 buffers / notify events
Iris->>PM: pm_runtime_put / adjust clocks/ICC (rgba(52,152,219,0.5))
V4L2->>App: buffer done / events
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
02
Squishy123
force-pushed
the
added-config-improvements-and-fixes-to-ayn-odin2
branch
3 times, most recently
from
35f5bb3 to
6d8c548
Compare
|
Is there a good way to credit the ROCKNIX devs - since a lot of the stuff came from their repo? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical 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 (11)
patch/kernel/archive/sm8550-6.13/0036_ASoC--qcom--sc8280xp-Add-support-for-Primary-I2S.patch (1)
35-55:⚠️ Potential issue | 🟠 MajorMissing error handling for
clk_prepare_enableand potential resource leak.Two issues in this function:
clk_prepare_enable()at line 46 can fail but its return value is ignored.- If
qcom_snd_sdw_startup()fails, the clock remains enabled (resource leak).🔧 Proposed fix with error handling
static int sc8280xp_snd_startup(struct snd_pcm_substream *substream) { unsigned int fmt = SND_SOC_DAIFMT_BP_FP; unsigned int codec_dai_fmt = SND_SOC_DAIFMT_BC_FC | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_I2S; struct snd_soc_pcm_runtime *rtd = substream->private_data; struct sc8280xp_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0); + int ret; switch (cpu_dai->id) { case PRIMARY_MI2S_RX: - clk_prepare_enable(pdata->i2s_clk); + ret = clk_prepare_enable(pdata->i2s_clk); + if (ret) + return ret; snd_soc_dai_set_fmt(cpu_dai, fmt); snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt); break; default: break; } - return qcom_snd_sdw_startup(substream); + ret = qcom_snd_sdw_startup(substream); + if (ret && cpu_dai->id == PRIMARY_MI2S_RX) + clk_disable_unprepare(pdata->i2s_clk); + + return ret; }patch/kernel/archive/sm8550-6.13/0033_leds--Add-driver-for-HEROIC-HTR3212.patch (3)
282-297:⚠️ Potential issue | 🔴 CriticalIncomplete GPIO error handling will crash on non-EPROBE_DEFER errors.
The code only handles
-EPROBE_DEFERfromdevm_gpiod_get(). If any other error occurs (e.g.,-ENOENT,-EIO),priv->sdbremains an error pointer, andgpiod_set_value_cansleep()on line 297 will dereference it, causing a kernel crash.Proposed fix
priv->sdb = devm_gpiod_get(dev, "sdb", GPIOD_OUT_HIGH); - if (PTR_ERR(priv->sdb) == -EPROBE_DEFER) - return -EPROBE_DEFER; + if (IS_ERR(priv->sdb)) + return dev_err_probe(dev, PTR_ERR(priv->sdb), + "Failed to get sdb GPIO\n");
292-311:⚠️ Potential issue | 🟠 MajorRegulator leak on probe failure paths.
If
htr3212_init_regs()orhtr3212_parse_dt()fails afterregulator_enable()succeeds, the regulator is not disabled before returning. This leaves the regulator enabled, causing a resource leak.Proposed fix using goto-based cleanup
ret = regulator_enable(priv->vdd); - if (ret < 0) { + if (ret < 0) return ret; - } gpiod_set_value_cansleep(priv->sdb, 1); usleep_range(10000, 11000); priv->client = client; i2c_set_clientdata(client, priv); ret = htr3212_init_regs(priv); if (ret) - return ret; + goto err_disable_regulator; ret = htr3212_parse_dt(dev, priv); if (ret) - return ret; + goto err_disable_regulator; return 0; + +err_disable_regulator: + gpiod_set_value_cansleep(priv->sdb, 0); + regulator_disable(priv->vdd); + return ret; }
348-348:⚠️ Potential issue | 🟡 MinorMissing closing angle bracket in MODULE_AUTHOR.
The email address is missing the closing
>.Proposed fix
-MODULE_AUTHOR("Teguh Sobirin <teguh@sobir.in"); +MODULE_AUTHOR("Teguh Sobirin <teguh@sobir.in>");patch/kernel/archive/sm8550-6.13/0031_input--Add-driver-for-RSInput-Gamepad.patch (5)
209-241:⚠️ Potential issue | 🟠 MajorStatic variable
prev_statescauses state sharing between device instances.The
static unsigned long prev_statesat line 211 is shared across all driver instances. If multiple gamepads are connected, they will share button state tracking, leading to incorrect input reports.Move
prev_statesintostruct rsinput_driver.Suggested fix
Add to the struct:
struct rsinput_driver { struct serdev_device *serdev; struct input_dev *input; struct gpio_desc *boot_gpio; struct gpio_desc *enable_gpio; struct gpio_desc *reset_gpio; uint8_t rx_buf[256]; uint8_t sequence_number; + unsigned long prev_states; };Then update handle_cmd_status:
static void handle_cmd_status(struct rsinput_driver *drv, const uint8_t *data, size_t payload_length) { if (payload_length >= 6) { - static unsigned long prev_states; unsigned long keys = data[FRAME_POS_DATA_1] | (data[FRAME_POS_DATA_2] << 8); unsigned long current_states = keys, changes; int i; - bitmap_xor(&changes, ¤t_states, &prev_states, ARRAY_SIZE(keymap)); + bitmap_xor(&changes, ¤t_states, &drv->prev_states, ARRAY_SIZE(keymap)); ... - prev_states = keys; + drv->prev_states = keys;
306-309:⚠️ Potential issue | 🟠 MajorRe-initializing MCU on every checksum failure is excessive and may block.
Calling
rsinput_init_commands()from the receive callback on checksum mismatch is problematic:
rsinput_init_commands()containsmsleep(100)calls, which may block inappropriately in the receive path- A single corrupted packet shouldn't trigger a full MCU re-initialization
- This could cause an initialization storm if line noise occurs
Consider simply discarding bad packets, or implementing a counter to reinit only after multiple consecutive failures.
Suggested fix
if (computed_checksum != received_checksum) { - rsinput_init_commands(drv); + dev_dbg_ratelimited(&serdev->dev, "Checksum mismatch, discarding packet\n"); goto error; }
333-352:⚠️ Potential issue | 🔴 CriticalGPIO error handling leaves error pointers that will cause kernel oops.
When
devm_gpiod_get_optional()fails, the error pointer is stored but not cleared. Later, checks likeif (drv->boot_gpio)evaluate to true for error pointers, andgpiod_set_value_cansleep()on an error pointer will crash the kernel.Either set the pointer to NULL on error or return the error from probe.
Suggested fix
drv->boot_gpio = devm_gpiod_get_optional(&serdev->dev, "boot", GPIOD_OUT_HIGH); if (IS_ERR(drv->boot_gpio)) { - error = PTR_ERR(drv->boot_gpio); - dev_warn(&serdev->dev, "Unable to get boot gpio: %d\n", error); + dev_warn(&serdev->dev, "Unable to get boot gpio: %ld\n", PTR_ERR(drv->boot_gpio)); + drv->boot_gpio = NULL; } drv->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_HIGH); if (IS_ERR(drv->enable_gpio)) { - error = PTR_ERR(drv->enable_gpio); - dev_warn(&serdev->dev, "Unable to get enable gpio: %d\n", error); + dev_warn(&serdev->dev, "Unable to get enable gpio: %ld\n", PTR_ERR(drv->enable_gpio)); + drv->enable_gpio = NULL; } drv->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(drv->reset_gpio)) { - error = PTR_ERR(drv->reset_gpio); - dev_warn(&serdev->dev, "Unable to get reset gpio: %d\n", error); + dev_warn(&serdev->dev, "Unable to get reset gpio: %ld\n", PTR_ERR(drv->reset_gpio)); + drv->reset_gpio = NULL; }
379-420:⚠️ Potential issue | 🔴 CriticalResource leak:
serdev_device_close()missing on error paths after open.After
serdev_device_open()succeeds at line 370, several error paths return without closing the serdev:
- Line 381: baudrate setup failure
- Line 387: input allocation failure
- Line 420: input registration failure
The serdev must be closed before returning on these error paths.
Suggested fix using goto-based cleanup
error = serdev_device_set_baudrate(serdev, 115200); - if (error < 0) - return dev_err_probe(&serdev->dev, error, "Failed to set up host baud rate\n"); + if (error < 0) { + dev_err_probe(&serdev->dev, error, "Failed to set up host baud rate\n"); + goto err_close; + } serdev_device_set_flow_control(serdev, false); drv->input = devm_input_allocate_device(&serdev->dev); - if (!drv->input) - return -ENOMEM; + if (!drv->input) { + error = -ENOMEM; + goto err_close; + } ... error = input_register_device(drv->input); - if (error) - return error; + if (error) + goto err_close; serdev_device_set_client_ops(serdev, &rsinput_rx_ops); error = rsinput_init_commands(drv); if (error < 0) { - serdev_device_close(serdev); - return error; + goto err_close; } return 0; + +err_close: + serdev_device_close(serdev); + return error; }
433-443:⚠️ Potential issue | 🟠 MajorRemove manual
input_unregister_device()for devm-managed input device.The input device is allocated with
devm_input_allocate_device(), which automatically handles cleanup including unregistration when the parent device is removed. Manually callinginput_unregister_device()on a devm-managed device can cause a double-free or use-after-free.Suggested fix
static void rsinput_remove(struct serdev_device *serdev) { struct rsinput_driver *drv = serdev_device_get_drvdata(serdev); serdev_device_close(serdev); - input_unregister_device(drv->input); if (drv->enable_gpio) gpiod_set_value_cansleep(drv->enable_gpio, 0); if (drv->reset_gpio) gpiod_set_value_cansleep(drv->reset_gpio, 0); }patch/kernel/archive/sm8550-6.13/0032_drm-panel--Add-panel-driver-for-Chipone-ICNA3512-b.patch (1)
286-298:⚠️ Potential issue | 🟠 MajorResource leak on error paths after PPS or compression mode failure.
If
mipi_dsi_picture_parameter_setormipi_dsi_compression_modefails, the function returns without disabling the regulators that were enabled at line 269. This leaves regulators enabled and the GPIO in reset state without proper cleanup.🔧 Proposed fix to add cleanup on error paths
ret = mipi_dsi_picture_parameter_set(pinfo->dsi, &pps); if (ret < 0) { dev_err(panel->dev, "failed to transmit PPS: %d\n", ret); - return ret; + goto err_disable_regulators; } /* Not required, ICNA3512 has DSC always enabled. */ ret = mipi_dsi_compression_mode(pinfo->dsi, true); if (ret < 0) { dev_err(panel->dev, "failed to enable compression mode: %d\n", ret); - return ret; + goto err_disable_regulators; } return 0; + +err_disable_regulators: + gpiod_set_value_cansleep(pinfo->reset_gpio, 1); + regulator_bulk_disable(ARRAY_SIZE(pinfo->supplies), pinfo->supplies); + return ret; }config/boards/ayn-odin2.csc (1)
100-113:⚠️ Potential issue | 🟡 MinorGuard firmware loops against missing directories.
If
/lib/firmware/qcom/sm8550*doesn’t exist, the glob can expand to a literal pattern andadd_firmwarewill be called with a non-existent path.🔧 Proposed fix
@@ - for f in /lib/firmware/qcom/sm8550/* ; do + for f in /lib/firmware/qcom/sm8550/* ; do + [ -e "$f" ] || continue add_firmware "${f#/lib/firmware/}" done - for f in /lib/firmware/qcom/sm8550/ayn/* ; do + for f in /lib/firmware/qcom/sm8550/ayn/* ; do + [ -e "$f" ] || continue add_firmware "${f#/lib/firmware/}" done
🤖 Fix all issues with AI agents
In
`@patch/kernel/archive/sm8550-6.13/0004_media--iris--introduce-iris-core-state-management-.patch`:
- Around line 252-260: The error cleanup after allocating core->sfr_vaddr
incorrectly calls dma_free_attrs(core->dev, sizeof(*q_tbl_hdr),
core->iface_q_table_vaddr, core->iface_q_table_daddr, DMA_ATTR_WRITE_COMBINE);
replace the size argument with the original allocation size (queue_size) used
for core->iface_q_table_vaddr so the full DMA region is freed; ensure the
dma_free_attrs call still uses the matching DMA_ATTR_WRITE_COMBINE flag and the
same buffer pointers core->iface_q_table_vaddr/core->iface_q_table_daddr.
In
`@patch/kernel/archive/sm8550-6.13/0007_media--iris--introduce-host-firmware-interface-wit.patch`:
- Around line 1066-1281: The response-path can call
iris_hfi_gen2_handle_system_error(core, NULL) on validation failure but
iris_hfi_gen2_handle_system_error dereferences pkt->type; modify
iris_hfi_gen2_handle_system_error to first check if pkt is NULL and handle that
case (log a generic system error message, set core->state = IRIS_CORE_ERROR and
schedule_delayed_work(&core->sys_error_handler, ...)) instead of dereferencing
pkt, leaving existing behavior for non-NULL pkt; this avoids changing callers
like iris_hfi_gen2_handle_response while ensuring safe NULL handling.
In
`@patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch`:
- Around line 1383-1417: The kzallocs for inst->fmt_src and inst->fmt_dst in the
initialization block must be checked for NULL to avoid kernel panics; after each
kzalloc (allocating inst->fmt_src and inst->fmt_dst) verify the pointer and on
failure free any prior allocation, set a proper error return (e.g. -ENOMEM) and
abort initialization so the caller (iris_open) can handle it; update the
function that contains these symbols (references: inst->fmt_src, inst->fmt_dst,
iris_get_buffer_size, iris_vpu_buf_count) to perform the NULL checks, free the
other fmt pointer if needed, and return an error code instead of continuing to
dereference.
In
`@patch/kernel/archive/sm8550-6.13/0014_media--iris--implement-iris-v4l2_ctrl_ops.patch`:
- Around line 219-240: The function iris_session_init_caps currently
dereferences platform caps without checks; update it to first validate
core->iris_platform_data, core->iris_platform_data->inst_fw_caps and
core->iris_platform_data->inst_fw_caps_size (and optionally core->inst_fw_caps)
and return early if any are NULL or size is 0 to avoid probe-time NULL derefs.
Locate iris_session_init_caps and add these guards before using caps, then
proceed with the existing loop that uses iris_valid_cap_id and fills
core->inst_fw_caps only when inputs are present.
In
`@patch/kernel/archive/sm8550-6.13/0019_media--iris--allocate`,-initialize-and-queue-intern.patch:
- Around line 79-106: The function iris_create_internal_buffer adds the newly
allocated struct iris_buffer (variable buffer) into buffers->list before calling
dma_alloc_attrs, and if dma_alloc_attrs fails the code returns -ENOMEM leaving
buffer in the list and leaking memory; fix iris_create_internal_buffer by
undoing the prior list_add_tail on allocation failure: call
list_del(&buffer->list) and kfree(buffer) (no dma_free needed since kvaddr is
NULL) before returning the error from the dma_alloc_attrs call.
In
`@patch/kernel/archive/sm8550-6.13/0023_media--iris--add-support-for-drain-sequence.patch`:
- Around line 392-396: The condition guarding STOP uses an equality test against
inst->sub_state (inst->sub_state != IRIS_INST_SUB_DRAIN) which is wrong for
bitflags; change the check to test the drain bit with a bitwise AND and negate
it (i.e. use !(inst->sub_state & IRIS_INST_SUB_DRAIN)) so STOP is blocked while
the IRIS_INST_SUB_DRAIN bit is set; update the branch that handles
V4L2_DEC_CMD_STOP and streaming (vb2_is_streaming(src_q)) to use this bitwise
test against inst->sub_state.
🟠 Major comments (15)
extensions/odin2-preset-firstrun.sh-41-48 (1)
41-48:⚠️ Potential issue | 🟠 MajorSecurity concern: Hardcoded weak default passwords.
Using
1234as default passwords for both root and user accounts is a security risk, especially if images are distributed publicly. Even if the first-run wizard prompts for password change, devices left unconfigured or images accessed before first boot are vulnerable.Consider using stronger randomly-generated defaults or requiring password setup during first-run without a preset.
extensions/odin2-preset-firstrun.sh-60-61 (1)
60-61:⚠️ Potential issue | 🟠 MajorDirectory will be owned by root, not the odin2 user.
The
pre_customize_image__hook runs during image build, but theodin2user is only preset (viaPRESET_USER_NAME) and won't be created until first boot. The cloned directory will be owned by root, causing permission issues when the user tries to access or modify their scripts.Fix by setting ownership after clone, or move this to a first-run hook that executes after user creation.
🐛 Proposed fix to set correct ownership
function pre_customize_image__add_odin2_scripts() { display_alert "Adding Odin2 Scripts" "${EXTENSION}" "info" chroot_sdcard mkdir -p /home/odin2/sys chroot_sdcard git clone https://github.com/Squishy123/odin2-scripts.git /home/odin2/sys/odin2-scripts + # Set ownership to uid/gid 1000 (default first user) since odin2 user doesn't exist yet + chroot_sdcard chown -R 1000:1000 /home/odin2 }extensions/odin2-preset-firstrun.sh-61-61 (1)
61-61:⚠️ Potential issue | 🟠 MajorSupply chain risk: Git clone without version pinning.
Cloning from an external repository without pinning to a specific commit hash or tag means builds are not reproducible and vulnerable to upstream changes or compromise.
Pin to a specific commit or tag for reproducible and secure builds.
🔒 Proposed fix to pin to a specific commit
- chroot_sdcard git clone https://github.com/Squishy123/odin2-scripts.git /home/odin2/sys/odin2-scripts + chroot_sdcard git clone --depth 1 --branch <TAG_OR_BRANCH> https://github.com/Squishy123/odin2-scripts.git /home/odin2/sys/odin2-scripts + # Or pin to specific commit: + # chroot_sdcard git clone https://github.com/Squishy123/odin2-scripts.git /home/odin2/sys/odin2-scripts + # chroot_sdcard git -C /home/odin2/sys/odin2-scripts checkout <COMMIT_HASH>patch/kernel/archive/sm8550-6.13/0018_media--iris--subscribe-parameters-and-properties-t.patch-95-111 (1)
95-111:⚠️ Potential issue | 🟠 MajorPotential buffer overflow if
change_param_sizeexceeds 31.The
payloadarray is fixed at 32 elements, but the loop copieschange_param_sizeelements starting at index 1. Ifchange_param_size > 31, this will overflow.While platform data sizes are typically small and controlled, adding a bounds check would be defensive. Same issue exists in
iris_hfi_gen2_subscribe_propertyat line 206.🛡️ Proposed defensive fix
+ if (change_param_size > ARRAY_SIZE(payload) - 1) { + dev_err(core->dev, "change_param_size exceeds payload capacity\n"); + return -EINVAL; + } + for (i = 0; i < change_param_size; i++) payload[i + 1] = change_param[i];patch/kernel/archive/sm8550-6.13/0019_media--iris--allocate,-initialize-and-queue-intern.patch-855-864 (1)
855-864:⚠️ Potential issue | 🟠 MajorLogic bug:
HFI_PORT_NONErejected forPERSISTbuffers.The first condition allows
HFI_PORT_NONEwhenbuffer_type == HFI_BUFFER_PERSIST, but the second condition then rejectsHFI_PORT_NONEbecause it's neitherHFI_PORT_BITSTREAMnorHFI_PORT_RAW. PERSIST buffers with PORT_NONE will incorrectly returnfalse.🐛 Proposed fix
static bool iris_hfi_gen2_is_valid_hfi_port(u32 port, u32 buffer_type) { + if (port == HFI_PORT_NONE && buffer_type == HFI_BUFFER_PERSIST) + return true; + if (port == HFI_PORT_NONE && buffer_type != HFI_BUFFER_PERSIST) return false; if (port != HFI_PORT_BITSTREAM && port != HFI_PORT_RAW) return false; return true; }patch/kernel/archive/sm8550-6.13/0024_media--iris--add-check-whether-the-video-session-i.patch-56-141 (1)
56-141:⚠️ Potential issue | 🟠 MajorProtect
core->instancestraversal withcore->lock.
iris_check_session_supported()walkscore->instanceswithout holding the lock, which can race with instance add/remove and lead to UAF or false negatives. Please wrap the iteration with the same lock used elsewhere in this file.🔒 Proposed fix
static int iris_check_session_supported(struct iris_inst *inst) { struct iris_core *core = inst->core; struct iris_inst *instance = NULL; bool found = false; int ret; - list_for_each_entry(instance, &core->instances, list) { - if (instance == inst) - found = true; - } + mutex_lock(&core->lock); + list_for_each_entry(instance, &core->instances, list) { + if (instance == inst) { + found = true; + break; + } + } + mutex_unlock(&core->lock);patch/kernel/archive/sm8550-6.13/0015_media--iris--implement-query_cap-ioctl.patch-29-35 (1)
29-35:⚠️ Potential issue | 🟠 MajorSet
device_capsandcapabilitiesiniris_querycap.Iris is a V4L2 mem2mem decoder and must advertise its capabilities. Without
device_capsandcapabilities, userspace cannot detect that the device supports M2M/streaming operations. Set these using the standard pattern for M2M decoders:Suggested fix
static int iris_querycap(struct file *filp, void *fh, struct v4l2_capability *cap) { strscpy(cap->driver, IRIS_DRV_NAME, sizeof(cap->driver)); strscpy(cap->card, "Iris Decoder", sizeof(cap->card)); + cap->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; }patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch-1462-1477 (1)
1462-1477:⚠️ Potential issue | 🟠 MajorSilent failure when max session count exceeded.
iris_add_sessionsilently fails to add the instance to the list when the maximum session count is reached. The caller iniris_open(line 1537) has no way to detect this failure, causing the open to appear successful while the session is not actually registered.This will cause
iris_get_instanceto return NULL for HFI responses targeting this session, leading to dropped responses or potential crashes.🐛 Proposed fix
-static void iris_add_session(struct iris_inst *inst) +static int iris_add_session(struct iris_inst *inst) { struct iris_core *core = inst->core; struct iris_inst *iter; u32 count = 0; mutex_lock(&core->lock); list_for_each_entry(iter, &core->instances, list) count++; - if (count < core->iris_platform_data->max_session_count) + if (count < core->iris_platform_data->max_session_count) { list_add_tail(&inst->list, &core->instances); + mutex_unlock(&core->lock); + return 0; + } mutex_unlock(&core->lock); + return -EBUSY; }And update the caller:
- iris_add_session(inst); + ret = iris_add_session(inst); + if (ret) { + iris_vdec_inst_deinit(inst); + goto fail_m2m_ctx_release; + }patch/kernel/archive/sm8550-6.13/0014_media--iris--implement-iris-v4l2_ctrl_ops.patch-446-474 (1)
446-474:⚠️ Potential issue | 🟠 MajorHandle cleanup when iris_ctrls_init() fails to avoid leaks.
On failure,fmt_src/fmt_dstallocations from earlier iniris_vdec_inst_init()are leaked because the error path iniris_open()doesn’t free them.🧹 Proposed fix
int iris_vdec_inst_init(struct iris_inst *inst) { struct iris_core *core = inst->core; struct v4l2_format *f; + int ret; /* ... existing init ... */ memcpy(&inst->fw_caps[0], &core->inst_fw_caps[0], INST_FW_CAP_MAX * sizeof(struct platform_inst_fw_cap)); - return iris_ctrls_init(inst); + ret = iris_ctrls_init(inst); + if (ret) { + kfree(inst->fmt_src); + inst->fmt_src = NULL; + kfree(inst->fmt_dst); + inst->fmt_dst = NULL; + } + return ret; }patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch-268-287 (1)
268-287:⚠️ Potential issue | 🟠 MajorProp-set failures are ignored; surface them to streamon.
Line 284-286 discards the return value from
cap->set, so firmware rejects can be silently ignored and streamon continues partially configured.Proposed fix (propagate errors)
- if (cap->cap_id && cap->set) - cap->set(inst, i); + if (cap->cap_id && cap->set) { + ret = cap->set(inst, i); + if (ret) + return ret; + }patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch-425-445 (1)
425-445:⚠️ Potential issue | 🟠 MajorAvoid unbalanced runtime‑PM reference count on resume failure.
The code calls
pm_runtime_put_sync()at the exit label unconditionally, butpm_runtime_resume_and_get()fails without incrementing the usage counter. This causes a refcount underflow when the resume fails. Return early on failed resume without calling anypm_runtime_put*(), and only put after a successful get.Proposed fix
- ret = pm_runtime_resume_and_get(core->dev); - if (ret < 0) - goto exit; + ret = pm_runtime_resume_and_get(core->dev); + if (ret < 0) + return ret; mutex_lock(&core->lock); ret = iris_hfi_queue_cmd_write_locked(core, pkt, pkt_size); - if (ret) { - mutex_unlock(&core->lock); - goto exit; - } mutex_unlock(&core->lock); + if (ret) { + pm_runtime_put_sync(core->dev); + return ret; + } pm_runtime_mark_last_busy(core->dev); pm_runtime_put_autosuspend(core->dev); return 0; - -exit: - pm_runtime_put_sync(core->dev); - - return ret;patch/kernel/archive/sm8550-6.13/0021_media--iris--add-support-for-dynamic-resolution-ch.patch-44-66 (1)
44-66:⚠️ Potential issue | 🟠 MajorFix DCI‑P3 mapping in
iris_hfi_get_v4l2_color_primaries.The switch statement receives an HFI primaries enum value but line 61 checks against
V4L2_COLORSPACE_DCI_P3(a V4L2 enum), so this case never matches. When the firmware sendsHFI_PRIMARIES_SMPTE_RP431_2, it falls through to the default case and incorrectly returnsV4L2_COLORSPACE_DEFAULT. Swap the constants so the HFI value maps to the correct V4L2 colorspace:Fix
- case V4L2_COLORSPACE_DCI_P3: - return HFI_PRIMARIES_SMPTE_RP431_2; + case HFI_PRIMARIES_SMPTE_RP431_2: + return V4L2_COLORSPACE_DCI_P3;patch/kernel/archive/sm8550-6.13/0040_phy--qcom--qmp-combo--register-a-typec-mux-to-chan.patch-51-136 (1)
51-136:⚠️ Potential issue | 🟠 MajorAddress the mux state comparison and USB3 detection logic.
TYPEC_MODE_USB3is the correct constant (defined ininclude/linux/usb/typec_altmode.h); there is noTYPEC_STATE_USB3.However, the logic has two issues:
Pre-mutex comparison (race condition): The early return
if (state->mode == qmp->mode)executes beforemutex_lock(), allowing concurrent modification ofqmp->modebetween the check and lock.Stale state used for USB3 detection: The USB3 check uses
qmp->mux_mode(the previously stored state before the current update on line 108) rather thanstate->mode(the incoming request). When a USB3 event arrives, this causes detection to use the previous mux state instead of the current request, delaying or skipping USB3-only mode activation until the next mode change.Additionally, when bailing out due to
new_mode == qmp->init_mode(line 96), onlyqmp->modeis updated, notqmp->mux_mode, leaving the stored state inconsistent.🔧 Suggested fixes
- if (state->mode == qmp->mode) - return 0; - - mutex_lock(&qmp->phy_mutex); + mutex_lock(&qmp->phy_mutex); + if (state->mode == qmp->mux_mode) + goto out; @@ - if (qmp->mux_mode == TYPEC_MODE_USB3) + if (state->mode == TYPEC_MODE_USB3) new_mode = QPHY_MODE_USB_ONLY; @@ if (new_mode == qmp->init_mode) { dev_dbg(qmp->dev, "typec_mux_set: same phy mode, bail out\n"); qmp->mode = state->mode; + qmp->mux_mode = state->mode; goto out; }patch/kernel/archive/sm8550-6.13/0007_media--iris--introduce-host-firmware-interface-wit.patch-78-110 (1)
78-110:⚠️ Potential issue | 🟠 MajorReinitialize
core_init_donebefore waiting for the system response.When
iris_core_init()is called again (e.g., during error recovery viairis_sys_error_handler()), thecore_init_donecompletion may already be signaled from the previous call. On re-entry,wait_for_completion_timeout()would return immediately even before the firmware responds to the new HFI commands, since the internal completion counter is already > 0.🔧 Proposed fix
@@ - ret = iris_hfi_core_init(core); + reinit_completion(&core->core_init_done); + ret = iris_hfi_core_init(core);config/boards/ayn-odin2-portal-grub.csc-55-61 (1)
55-61:⚠️ Potential issue | 🟠 MajorFile mode
655prevents owner from executing scripts.Mode
655(rw-r-xr-x) denies execute permission to the owner while granting it to group/others. For executable scripts, use755(rwxr-xr-x).Proposed fix
- install -Dm655 $SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network.sh $destination/usr/local/bin/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ + install -Dm755 $SRC/packages/bsp/usb-gadget-network/setup-usbgadget-network.sh $destination/usr/local/bin/ + install -Dm755 $SRC/packages/bsp/usb-gadget-network/remove-usbgadget-network.sh $destination/usr/local/bin/ install -Dm644 $SRC/packages/bsp/usb-gadget-network/usbgadget-rndis.service $destination/usr/lib/systemd/system/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook $destination/etc/initramfs-tools/hooks/usb-gadget - install -Dm655 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount $destination/etc/initramfs-tools/scripts/init-premount/usb-gadget - install -Dm655 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ - install -Dm655 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/ + install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-hook $destination/etc/initramfs-tools/hooks/usb-gadget + install -Dm755 $SRC/packages/bsp/usb-gadget-network/usb-gadget-initramfs-premount $destination/etc/initramfs-tools/scripts/init-premount/usb-gadget + install -Dm755 $SRC/packages/bsp/usb-gadget-network/dropbear $destination/etc/initramfs-tools/scripts/init-premount/ + install -Dm755 $SRC/packages/bsp/usb-gadget-network/kill-dropbear $destination/etc/initramfs-tools/scripts/init-bottom/
🟡 Minor comments (17)
extensions/odin2-preset-firstrun.sh-54-55 (1)
54-55:⚠️ Potential issue | 🟡 MinorIncomplete implementation: "clone starter scripts" comment with no code.
The comment suggests functionality that was intended but not implemented. Either remove the placeholder comment or implement the missing functionality.
patch/kernel/archive/sm8550-6.13/0018_media--iris--subscribe-parameters-and-properties-t.patch-99-103 (1)
99-103:⚠️ Potential issue | 🟡 MinorInconsistent error handling: logs error but returns success.
When the properties are already set for the given plane,
dev_err()is called but the function returns0(success). This is confusing - either it's an expected condition (remove the error log) or an actual error (return-EINVALor-EALREADY).💡 Suggested fix - treat as no-op without error log
if ((V4L2_TYPE_IS_OUTPUT(plane) && inst_hfi_gen2->ipsc_properties_set) || (V4L2_TYPE_IS_CAPTURE(plane) && inst_hfi_gen2->opsc_properties_set)) { - dev_err(core->dev, "invalid plane\n"); return 0; }patch/kernel/archive/sm8550-6.13/0003_media--iris--implement-iris-v4l2-file-ops.patch-198-207 (1)
198-207:⚠️ Potential issue | 🟡 MinorTypo in comment: "seralise" should be "serialize".
Minor typo in the struct documentation comment.
- * `@lock`: lock to seralise forward and reverse threads + * `@lock`: lock to serialize forward and reverse threadspatch/kernel/archive/sm8550-6.13/0029_arm64--dts--qcom--sm8550--Add-iris-video-codec-nod.patch-94-94 (1)
94-94:⚠️ Potential issue | 🟡 MinorMinor: Trailing whitespace on this line.
There appears to be trailing whitespace before the
videoccnode. While this won't affect functionality, it's a minor code hygiene issue.patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch-168-211 (1)
168-211:⚠️ Potential issue | 🟡 MinorError code from first HFI command is overwritten.
In the stop/release sequence (lines 183-191), if
HFI_CMD_SESSION_STOPfails butHFI_CMD_SESSION_RELEASE_RESOURCESsucceeds, the original error is lost. Similarly, the secondretassignment unconditionally overwrites the first.Consider preserving the first error:
Proposed fix
reinit_completion(&inst->completion); iris_hfi_gen1_packet_session_cmd(inst, &pkt, HFI_CMD_SESSION_STOP); ret = iris_hfi_queue_cmd_write(core, &pkt, pkt.shdr.hdr.size); if (!ret) ret = iris_wait_for_session_response(inst, false); reinit_completion(&inst->completion); iris_hfi_gen1_packet_session_cmd(inst, &pkt, HFI_CMD_SESSION_RELEASE_RESOURCES); - ret = iris_hfi_queue_cmd_write(core, &pkt, pkt.shdr.hdr.size); - if (!ret) - ret = iris_wait_for_session_response(inst, false); + if (!ret) { + ret = iris_hfi_queue_cmd_write(core, &pkt, pkt.shdr.hdr.size); + if (!ret) + ret = iris_wait_for_session_response(inst, false); + }patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch-649-654 (1)
649-654:⚠️ Potential issue | 🟡 MinorLogging bug: prints same value for old and new state.
The
dev_dbglogsinst->stateafter it's already been assignedrequest_state, so both values in the log message will be identical.Proposed fix
+ enum iris_inst_state old_state = inst->state; + change_state: inst->state = request_state; dev_dbg(inst->core->dev, "state changed from %x to %x\n", - inst->state, request_state); + old_state, request_state); return 0;patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch-1275-1281 (1)
1275-1281:⚠️ Potential issue | 🟡 MinorDefault return for invalid buffer types may mask bugs.
iris_v4l2_type_to_driverreturnsBUF_OUTPUTfor any type that isn'tV4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, including invalid types. This could silently hide bugs where an unexpected type is passed.🛡️ Suggested defensive check
static inline enum iris_buffer_type iris_v4l2_type_to_driver(u32 type) { if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) return BUF_INPUT; - else + else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) return BUF_OUTPUT; + else + return BUF_TYPE_MAX; /* Invalid type indicator */ }Callers should then check for
BUF_TYPE_MAXas an error.patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch-1552-1566 (1)
1552-1566:⚠️ Potential issue | 🟡 MinorSession close called unconditionally regardless of whether session was opened.
iris_session_closeis always called iniris_close, but if the VB2 queue was never set up (e.g., open succeeded but no reqbufs was issued),session_openwas never called (once_per_session_setremains false).For gen2, this is handled by the NULL check on
inst_hfi_gen2->packet(line 754), but for gen1, a close command is sent to firmware for a session that was never opened.🛡️ Proposed guard
static void iris_session_close(struct iris_inst *inst) { const struct iris_hfi_command_ops *hfi_ops = inst->core->hfi_ops; bool wait_for_response = true; int ret; + if (!inst->once_per_session_set) + return; + reinit_completion(&inst->completion); ret = hfi_ops->session_close(inst);patch/kernel/archive/sm8550-6.13/0010_media--iris--implement-s_fmt,-g_fmt-and-try_fmt-io.patch-50-91 (1)
50-91:⚠️ Potential issue | 🟡 Minor
TRY_FMTmust return aligned dimensions consistent withS_FMT.
V4L2 spec requiresVIDIOC_TRY_FMTto return the exact same adjusted format thatVIDIOC_S_FMTwould accept. Currently,try_fmt()returns unaligned width/height, whiles_fmt()aligns to codec/stride constraints (OUTPUT: 16-byte alignment; CAPTURE: 128×32 with 128-byte stride). This mismatch misleads userspace on buffer sizing. Add the proposed alignment totry_fmt()to matchs_fmt().🔧 Proposed fix
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) { f_inst = inst->fmt_src; f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; } + f->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, DEFAULT_CODEC_ALIGNMENT); + f->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, DEFAULT_CODEC_ALIGNMENT); break; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) { f_inst = inst->fmt_dst; f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; } @@ if (vb2_is_streaming(src_q)) { f_inst = inst->fmt_src; f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; } + f->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128); + f->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32); + f->fmt.pix_mp.plane_fmt[0].bytesperline = ALIGN(f->fmt.pix_mp.width, 128); break;patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch-634-661 (1)
634-661:⚠️ Potential issue | 🟡 MinorFix unsigned integer underflow in bandwidth delta check.
The expression
abs(bw_kbps - bw_prev)operates onunsigned longvalues. Whenbw_kbps < bw_prev, the subtraction underflows to a large positive value due to unsigned arithmetic, causing the threshold check to behave incorrectly. Additionally,abs()expects a signedintargument. Compute the delta explicitly by choosing the correct subtraction direction.🛠️ Proposed fix
- if (abs(bw_kbps - bw_prev) < BW_THRESHOLD && bw_prev) - return ret; + if (bw_prev) { + unsigned long diff = (bw_kbps > bw_prev) ? + (bw_kbps - bw_prev) : + (bw_prev - bw_kbps); + if (diff < BW_THRESHOLD) + return ret; + }patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch-677-689 (1)
677-689:⚠️ Potential issue | 🟡 MinorBalance
pm_runtime_get_sync()on failure.
pm_runtime_get_sync()increments the device's usage counter before attempting the resume; if the resume fails, the usage counter is not automatically decremented, causing a leak. Addpm_runtime_put_noidle()on error to balance the count.🛠️ Proposed fix
ret = pm_runtime_get_sync(pd_dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_noidle(pd_dev); return ret; + }patch/kernel/archive/sm8550-6.13/0005_media--iris--implement-video-firmware-load-unload.patch-181-185 (1)
181-185:⚠️ Potential issue | 🟡 MinorError code overwritten with
-ENOMEM.Line 185 always returns
-ENOMEMregardless of the actual error returned byiris_load_fw_to_memory(). This loses the original error information (could be-EINVAL,-EIO, etc.).🐛 Proposed fix
ret = iris_load_fw_to_memory(core, fwpath); if (ret) { dev_err(core->dev, "firmware download failed\n"); - return -ENOMEM; + return ret; }patch/kernel/archive/sm8550-6.13/0005_media--iris--implement-video-firmware-load-unload.patch-147-149 (1)
147-149:⚠️ Potential issue | 🟡 MinorMissing return value assignment on
memremapfailure.When
memremap()returns NULL at line 148, the code jumps toerr_release_fwbutretis not set, leaving it with its previous value (0 from successfulrequest_firmware). This causes the function to return success (0) despite the failure.🐛 Proposed fix
mem_virt = memremap(mem_phys, res_size, MEMREMAP_WC); - if (!mem_virt) + if (!mem_virt) { + ret = -ENOMEM; goto err_release_fw; + }patch/kernel/archive/sm8550-6.13/0007_media--iris--introduce-host-firmware-interface-wit.patch-1286-1461 (1)
1286-1461:⚠️ Potential issue | 🟡 MinorPropagate queue write errors instead of masking them.
Any non-zero return from
iris_hfi_queue_write()is treated as “queue full” and mapped to-ENODATA, hiding-EINVALand other failures.🔧 Proposed fix
@@ int iris_hfi_queue_cmd_write_locked(struct iris_core *core, void *pkt, u32 pkt_size) { struct iris_iface_q_info *q_info = &core->command_queue; + int ret; if (core->state == IRIS_CORE_ERROR) return -EINVAL; - if (!iris_hfi_queue_write(q_info, pkt, pkt_size)) { - iris_vpu_raise_interrupt(core); - } else { - dev_err(core->dev, "queue full\n"); - return -ENODATA; - } - - return 0; + ret = iris_hfi_queue_write(q_info, pkt, pkt_size); + if (ret) { + dev_err(core->dev, "queue write failed: %d\n", ret); + return ret; + } + iris_vpu_raise_interrupt(core); + return 0; }patch/kernel/archive/sm8550-6.13/0004_media--iris--introduce-iris-core-state-management-.patch-519-530 (1)
519-530:⚠️ Potential issue | 🟡 MinorMutex not destroyed on probe error paths.
mutex_init(&core->lock)is called early in probe (line 545), but if any subsequent step fails (e.g.,dma_set_mask_and_coherent), the error path jumps to labels that don't callmutex_destroy. The mutex is only destroyed iniris_remove.This is a minor resource leak since
mutex_destroyis mostly a debug aid on Linux, but it's worth noting for completeness.config/boards/ayn-odin2-portal-grub.csc-32-46 (1)
32-46:⚠️ Potential issue | 🟡 MinorSubshell exit does not propagate failure to the caller.
If
cdfails (line 39),exit 6only exits the subshell, and the outer function continues silently. Consider using|| return 6outside the subshell or checking the subshell's exit status.Additionally, the download lacks integrity verification (e.g., checksum). Consider pinning to a specific commit SHA and verifying the download.
Proposed fix to propagate subshell failure
function pre_customize_image__ayn-odin2_alsa_ucm_conf() { if ! ayn-odin2portal_is_userspace_supported; then return 0 fi display_alert "Add alsa-ucm-conf for ${BOARD}" "${RELEASE}" "warn" ( cd "${SDCARD}/usr/share/alsa" || exit 6 curl -L -o temp.zip "https://github.com/AYNTechnologies/alsa-ucm-conf/archive/refs/heads/ayn/v1.2.13.zip" unzip -o temp.zip unzip_dir=$(unzip -Z1 temp.zip | head -n1 | cut -d/ -f1) cp -rf "${unzip_dir}/"* . rm -rf "$unzip_dir" temp.zip - ) + ) || return $? }config/boards/ayn-odin2-portal-grub.csc-74-81 (1)
74-81:⚠️ Potential issue | 🟡 MinorMissing second argument to
display_alerton lines 75 and 79.Other calls use three arguments (message, detail, level). These calls only have message and level, which may cause incorrect output.
Proposed fix
- display_alert "Adding Mesa PPA For Ubuntu ${BOARD}" "warn" + display_alert "Adding Mesa PPA For Ubuntu ${BOARD}" "${RELEASE}" "warn" do_with_retries 3 chroot_sdcard add-apt-repository ppa:kisak/kisak-mesa --yes do_with_retries 3 chroot_sdcard_apt_get_update - display_alert "Installing Mesa Vulkan Drivers" + display_alert "Installing Mesa Vulkan Drivers" "${RELEASE}" "info"
patch/kernel/archive/sm8550-6.13/0004_media--iris--introduce-iris-core-state-management-.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0007_media--iris--introduce-host-firmware-interface-wit.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0014_media--iris--implement-iris-v4l2_ctrl_ops.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0019_media--iris--allocate,-initialize-and-queue-intern.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0023_media--iris--add-support-for-drain-sequence.patch
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@config/boards/ayn-odin2-grub.csc`:
- Around line 67-110: The post_family_tweaks__ayn-odin2_enable_services function
currently unconditionally moves armbian.sources.disabled which can fail under
errexit and enables qbootctl.service twice; change the mv operations to be
guarded (e.g. test existence: [[ -f
"${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" ]] && mv ... ) and
similarly guard the restore move, and remove the duplicate chroot_sdcard
systemctl enable qbootctl.service so qbootctl.service is enabled only once;
update references to the exact filenames
("${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" and
"armbian.sources") and the command chroot_sdcard systemctl enable
qbootctl.service in the function.
🧹 Nitpick comments (1)
config/boards/ayn-odin2-grub.csc (1)
112-142: Avoid glob expansion errors in the initramfs hook when directories are missing.
Usenullglobso missing firmware directories don’t pass literal globs intoadd_firmware.🧩 Suggested hardening
. /usr/share/initramfs-tools/hook-functions + shopt -s nullglob for f in /lib/firmware/qcom/sm8550/* ; do add_firmware "${f#/lib/firmware/}" done @@ for f in /lib/firmware/qca/* ; do add_firmware "${f#/lib/firmware/}" done + shopt -u nullglob
Squishy123
force-pushed
the
added-config-improvements-and-fixes-to-ayn-odin2
branch
from
a4c5f01 to
03c0034
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/boards/ayn-odin2.csc (1)
84-92:⚠️ Potential issue | 🟡 MinorGuard Armbian sources toggling to prevent build failures.
If
armbian.sources.disableddoesn't exist, themvcommand will fail undererrexit. This is the same issue flagged in the GRUB variant.🛠️ Suggested fix
# We need unudhcpd from armbian repo, so enable it - mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources + if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" ]]; then + mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" "${SDCARD}/etc/apt/sources.list.d/armbian.sources" + fi do_with_retries 3 chroot_sdcard_apt_get_update display_alert "Installing ${BOARD} tweaks" "warn" do_with_retries 3 chroot_sdcard_apt_get_install alsa-ucm-conf qbootctl qrtr-tools unudhcpd mkbootimg # disable armbian repo back - mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled + if [[ -f "${SDCARD}/etc/apt/sources.list.d/armbian.sources" ]]; then + mv "${SDCARD}/etc/apt/sources.list.d/armbian.sources" "${SDCARD}/etc/apt/sources.list.d/armbian.sources.disabled" + fi
🤖 Fix all issues with AI agents
In `@config/boards/ayn-odin2-portal-grub.csc`:
- Around line 89-97: The two bare mv operations that toggle armbian.sources can
fail under errexit if the files are missing; wrap both moves with existence
checks (e.g., test -e before mv) or use safe mv semantics and ensure the
disable-back step always runs (place it in a cleanup/finally path or trap) so
package installation (do_with_retries / chroot_sdcard_apt_get_install) cannot
cause the script to exit with the armbian repo left enabled; update the block
that references "${SDCARD}"/etc/apt/sources.list.d/armbian.sources and
armbian.sources.disabled accordingly.
🧹 Nitpick comments (2)
config/boards/ayn-odin2-grub.csc (1)
32-46: Consider error handling for network operations.The
curlandunzipcommands could fail silently within the subshell. Consider addingset -eor explicit error checks.♻️ Optional improvement
( + set -e cd "${SDCARD}/usr/share/alsa" || exit 6 curl -L -o temp.zip "https://github.com/AYNTechnologies/alsa-ucm-conf/archive/refs/heads/ayn/v1.2.13.zip" + [[ -f temp.zip ]] || { echo "Failed to download alsa-ucm-conf"; exit 1; } unzip -o temp.zipconfig/boards/ayn-odin2-portal-grub.csc (1)
32-46: Consider error handling for network operations.Same as the non-Portal variant:
curlandunzipcommands could fail silently.♻️ Optional improvement
( + set -e cd "${SDCARD}/usr/share/alsa" || exit 6 curl -L -o temp.zip "https://github.com/AYNTechnologies/alsa-ucm-conf/archive/refs/heads/ayn/v1.2.13.zip" + [[ -f temp.zip ]] || { echo "Failed to download alsa-ucm-conf"; exit 1; } unzip -o temp.zip
|
hmm im wondering if maybe theres a better way for us to manage the grub images? any thoughts? |
🚫 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. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@config/boards/ayn-odin2-portal.csc`:
- Around line 50-56: The install commands in the diff use mode 655 for
executable scripts (e.g. setup-usbgadget-network.sh,
remove-usbgadget-network.sh, usb-gadget-initramfs-hook,
usb-gadget-initramfs-premount, dropbear, kill-dropbear) which prevents the owner
from executing them; change those install -Dm655 invocations to install -Dm755
so owner/group/others have the expected executable bit set for these scripts and
initramfs hooks.
- Around line 33-40: The download/unpack block around cd
"${SDCARD}/usr/share/alsa", curl -L -o temp.zip, unzip -o temp.zip and the
unzip_dir logic must propagate failures to the main script and handle bad
downloads: remove or change the subshell so a failing cd actually aborts the
script (or explicitly check cd and call exit), add curl -f (and check its exit
status) when fetching temp.zip, verify unzip succeeded (check its exit code and
that unzip_dir is non-empty) before copying, and ensure cleanup and a non-zero
exit from the main script on any of these failures so errors aren’t swallowed.
🧹 Nitpick comments (2)
config/boards/ayn-odin2-portal.csc (2)
69-82: Inconsistentdisplay_alertcalls and opportunity to consolidate Mesa installation.
Lines 70, 74, 80, 88 have inconsistent
display_alertarguments—some are missing the level parameter (third argument).The Mesa driver installation is duplicated between Ubuntu (lines 73-75) and Trixie (lines 79-81) blocks.
Proposed consolidation
- if [[ "${RELEASE}" == "jammy" ]] || [[ "${RELEASE}" == "noble" ]] || [[ "${RELEASE}" == "plucky" ]]; then - display_alert "Adding Mesa PPA For Ubuntu ${BOARD}" "warn" + if [[ "${RELEASE}" == "jammy" || "${RELEASE}" == "noble" || "${RELEASE}" == "plucky" ]]; then + display_alert "Adding Mesa PPA for ${BOARD}" "${RELEASE}" "info" do_with_retries 3 chroot_sdcard add-apt-repository ppa:kisak/kisak-mesa --yes - - do_with_retries 3 chroot_sdcard_apt_get_update - display_alert "Installing Mesa Vulkan Drivers" - do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-tools fi - if [[ "${RELEASE}" == "trixie" ]]; then - do_with_retries 3 chroot_sdcard_apt_get_update - display_alert "Installing Mesa Vulkan Drivers" - do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-tools - fi + do_with_retries 3 chroot_sdcard_apt_get_update + display_alert "Installing Mesa Vulkan Drivers" "${RELEASE}" "info" + do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-tools
85-91: Add existence check before renaming Armbian sources file.If
armbian.sources.disableddoesn't exist, themvcommand will fail. Consider adding a guard:Proposed fix
# We need unudhcpd from armbian repo, so enable it - mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources + [[ -f "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled ]] && \ + mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled "${SDCARD}"/etc/apt/sources.list.d/armbian.sources do_with_retries 3 chroot_sdcard_apt_get_update display_alert "Installing ${BOARD} tweaks" "warn" do_with_retries 3 chroot_sdcard_apt_get_install alsa-ucm-conf qbootctl qrtr-tools unudhcpd mkbootimg # disable armbian repo back - mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled + [[ -f "${SDCARD}"/etc/apt/sources.list.d/armbian.sources ]] && \ + mv "${SDCARD}"/etc/apt/sources.list.d/armbian.sources "${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled
Squishy123
force-pushed
the
added-config-improvements-and-fixes-to-ayn-odin2
branch
from
e860cbd to
5168111
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
patch/kernel/archive/sm8550-6.13/0036_ASoC--qcom--sc8280xp-Add-support-for-Primary-I2S.patch (1)
35-55:⚠️ Potential issue | 🟠 MajorHandle clock and DAI-format failures in startup.
The return codes from
clk_prepare_enable()(line 46), bothsnd_soc_dai_set_fmt()calls (lines 47–48), andqcom_snd_sdw_startup()are ignored. This creates multiple failure paths with no error recovery:
- Clock enable can fail (provider not ready, -EPROBE_DEFER, etc.) but is never checked; if a subsequent operation fails, the clock is left enabled (resource leak).
- Format configuration can fail with -EINVAL or other errors, silently continuing with misconfigured DAI.
qcom_snd_sdw_startup()failure leaves the clock enabled with no cleanup path.Propagate all errors and unwind on failure, including cleanup of the enabled clock on error paths.
Suggested fix
static int sc8280xp_snd_startup(struct snd_pcm_substream *substream) { unsigned int fmt = SND_SOC_DAIFMT_BP_FP; unsigned int codec_dai_fmt = SND_SOC_DAIFMT_BC_FC | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_I2S; struct snd_soc_pcm_runtime *rtd = substream->private_data; struct sc8280xp_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0); + int ret; + bool i2s_clk_enabled = false; switch (cpu_dai->id) { case PRIMARY_MI2S_RX: - clk_prepare_enable(pdata->i2s_clk); - snd_soc_dai_set_fmt(cpu_dai, fmt); - snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt); + ret = clk_prepare_enable(pdata->i2s_clk); + if (ret) + return ret; + i2s_clk_enabled = true; + + ret = snd_soc_dai_set_fmt(cpu_dai, fmt); + if (ret) + goto err_clk; + + ret = snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt); + if (ret) + goto err_clk; break; default: break; } - return qcom_snd_sdw_startup(substream); + ret = qcom_snd_sdw_startup(substream); + if (ret) + goto err_clk; + return 0; + +err_clk: + if (i2s_clk_enabled) + clk_disable_unprepare(pdata->i2s_clk); + return ret; }patch/kernel/archive/sm8550-6.13/0032_drm-panel--Add-panel-driver-for-Chipone-ICNA3512-b.patch (3)
111-131:⚠️ Potential issue | 🟠 MajorGuard against NULL connector before dereference.
pinfo->connectorcan be unset beforeget_modes()runs, which would cause a NULL deref here.🐛 Proposed fix
static int icna3512_get_current_mode(struct panel_info *pinfo) { struct drm_connector *connector = pinfo->connector; struct drm_crtc_state *crtc_state; int i; /* Return the default (first) mode if no info available yet */ - if (!connector->state || !connector->state->crtc) + if (!connector || !connector->state || !connector->state->crtc || + !connector->state->crtc->state) return 0; crtc_state = connector->state->crtc->state;
263-300:⚠️ Potential issue | 🟠 MajorDisable regulators on all prepare failure paths.
If PPS/compression setup fails, regulators stay enabled, leaving the panel powered despite a failed prepare.🔧 Proposed fix
ret = pinfo->desc->init_sequence(pinfo); if (ret < 0) { - regulator_bulk_disable(ARRAY_SIZE(pinfo->supplies), pinfo->supplies); dev_err(panel->dev, "failed to initialize panel: %d\n", ret); - return ret; + goto err_disable; } drm_dsc_pps_payload_pack(&pps, &pinfo->desc->dsc); ret = mipi_dsi_picture_parameter_set(pinfo->dsi, &pps); if (ret < 0) { dev_err(panel->dev, "failed to transmit PPS: %d\n", ret); - return ret; + goto err_disable; } /* Not required, ICNA3512 has DSC always enabled. */ ret = mipi_dsi_compression_mode(pinfo->dsi, true); if (ret < 0) { dev_err(panel->dev, "failed to enable compression mode: %d\n", ret); - return ret; + goto err_disable; } return 0; + +err_disable: + regulator_bulk_disable(ARRAY_SIZE(pinfo->supplies), pinfo->supplies); + return ret;
392-424:⚠️ Potential issue | 🟡 MinorRestore LPM mode_flags on error paths.
If the DCS call fails,mode_flagsstays in non‑LPM, which can affect power behavior.🧯 Proposed fix
static int icna3512_bl_update_status(struct backlight_device *bl) { struct mipi_dsi_device *dsi = bl_get_data(bl); u16 brightness = backlight_get_brightness(bl); + unsigned long mode_flags = dsi->mode_flags; int ret; dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); - if (ret < 0) - return ret; - - dsi->mode_flags |= MIPI_DSI_MODE_LPM; - - return 0; + dsi->mode_flags = mode_flags; + return ret; } static int icna3512_bl_get_brightness(struct backlight_device *bl) { struct mipi_dsi_device *dsi = bl_get_data(bl); u16 brightness; + unsigned long mode_flags = dsi->mode_flags; int ret; dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness); - if (ret < 0) - return ret; - - dsi->mode_flags |= MIPI_DSI_MODE_LPM; + dsi->mode_flags = mode_flags; + if (ret < 0) + return ret; return brightness; }patch/kernel/archive/sm8550-6.13/0031_input--Add-driver-for-RSInput-Gamepad.patch (5)
22-24:⚠️ Potential issue | 🟡 MinorPolish Kconfig prompt grammar.
Line 23’s prompt string is awkward; please rephrase for clarity.
✏️ Suggested wording
config JOYSTICK_RSINPUT - tristate "UART Based gamepad driver that found in AYN and Retroid Pocket products" + tristate "UART-based gamepad driver found in AYN and Retroid Pocket products" depends on SERIAL_DEV_BUS
101-109:⚠️ Potential issue | 🟠 MajorMake
prev_statesper-device (avoid static cross-talk).Line 211 uses a
staticbitmask shared across all instances; multiple gamepads will interfere with each other’s key-change detection. Store it instruct rsinput_driverinstead.✅ Per-device state
struct rsinput_driver { struct serdev_device *serdev; struct input_dev *input; struct gpio_desc *boot_gpio; struct gpio_desc *enable_gpio; struct gpio_desc *reset_gpio; uint8_t rx_buf[256]; uint8_t sequence_number; + unsigned long prev_states; }; @@ - static unsigned long prev_states; unsigned long keys = data[FRAME_POS_DATA_1] | (data[FRAME_POS_DATA_2] << 8); unsigned long current_states = keys, changes; int i; - bitmap_xor(&changes, ¤t_states, &prev_states, ARRAY_SIZE(keymap)); + bitmap_xor(&changes, ¤t_states, &drv->prev_states, + ARRAY_SIZE(keymap)); @@ - prev_states = keys; + drv->prev_states = keys;Also applies to: 209-237
333-369:⚠️ Potential issue | 🔴 CriticalKernel crash: ERR_PTR not cleared before passing to gpiod_set_value_cansleep().
Lines 333–352 log errors from
devm_gpiod_get_optional()withdev_warn()but do not clear the ERR_PTR. Lines 354–369 then checkif (drv->boot_gpio)and similar conditions, which evaluate true for ERR_PTR (non-zero pointer), causing a crash when passed togpiod_set_value_cansleep(). This function is not ERR_PTR-safe and will dereference the invalid pointer.Fix by returning an error via
dev_err_probe()(which properly handles-EPROBE_DEFER) or by nulling the pointer in the error path before later use.🛡️ Safer error handling
- drv->boot_gpio = - devm_gpiod_get_optional(&serdev->dev, "boot", GPIOD_OUT_HIGH); - if (IS_ERR(drv->boot_gpio)) { - error = PTR_ERR(drv->boot_gpio); - dev_warn(&serdev->dev, "Unable to get boot gpio: %d\n", error); - } + drv->boot_gpio = devm_gpiod_get_optional(&serdev->dev, "boot", + GPIOD_OUT_HIGH); + if (IS_ERR(drv->boot_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->boot_gpio), + "Unable to get boot gpio\n"); - drv->enable_gpio = - devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_HIGH); - if (IS_ERR(drv->enable_gpio)) { - error = PTR_ERR(drv->enable_gpio); - dev_warn(&serdev->dev, "Unable to get enable gpio: %d\n", error); - } + drv->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable", + GPIOD_OUT_HIGH); + if (IS_ERR(drv->enable_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->enable_gpio), + "Unable to get enable gpio\n"); - drv->reset_gpio = - devm_gpiod_get_optional(&serdev->dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(drv->reset_gpio)) { - error = PTR_ERR(drv->reset_gpio); - dev_warn(&serdev->dev, "Unable to get reset gpio: %d\n", error); - } + drv->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset", + GPIOD_OUT_HIGH); + if (IS_ERR(drv->reset_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->reset_gpio), + "Unable to get reset gpio\n");
281-311:⚠️ Potential issue | 🟠 MajorHandle partial frames by buffering across
receive_bufcallbacks.The
rsinput_rxfunction overwritesdrv->rx_bufon each callback withmemcpy(drv->rx_buf, data, count)and does not accumulate incomplete frames. When serdev delivers data in chunks smaller than a complete frame (MCU_PKT_SIZE_MIN = 9 bytes), partial frames are lost:
- Callback receives incomplete frame →
rsinput_process_datadetects incomplete frame (line 246) and returns- Next callback overwrites buffer → previous partial frame data is lost
- Trailing bytes from
rsinput_process_data(line 277) are warned about but discardedAdd an
rx_lenfield to track buffered data and preserve trailing bytes across callbacks. Buffer incoming data and only parse when a complete frame is available.
370-438:⚠️ Potential issue | 🟠 MajorUnwind serdev device on probe errors.
Lines 370–427: After
serdev_device_open()succeeds, failures at lines 376 (baudrate), 409 (input allocation), and 413 (input registration) return without closing the UART device. Redirect these paths to a common error label that closes serdev before returning.Note: The input device is allocated with
devm_input_allocate_device(), so devres automatically handles unregistration and cleanup on probe failure. Theinput_unregister_device()call inrsinput_remove()is redundant and can be removed.🔧 Suggested error-unwind pattern
error = serdev_device_set_baudrate(serdev, 115200); - if (error < 0) - return dev_err_probe(&serdev->dev, error, - "Failed to set up host baud rate\n"); + if (error < 0) { + error = dev_err_probe(&serdev->dev, error, + "Failed to set up host baud rate\n"); + goto err_close; + } serdev_device_set_flow_control(serdev, false); drv->input = devm_input_allocate_device(&serdev->dev); - if (!drv->input) - return -ENOMEM; + if (!drv->input) { + error = -ENOMEM; + goto err_close; + } @@ - error = input_register_device(drv->input); - if (error) - return error; + error = input_register_device(drv->input); + if (error) + goto err_close; @@ error = rsinput_init_commands(drv); - if (error < 0) { - serdev_device_close(serdev); - return error; - } + if (error < 0) + goto err_close; return 0; +err_close: + serdev_device_close(serdev); + return error; @@ static void rsinput_remove(struct serdev_device *serdev) { struct rsinput_driver *drv = serdev_device_get_drvdata(serdev); serdev_device_close(serdev); - input_unregister_device(drv->input); if (drv->enable_gpio) gpiod_set_value_cansleep(drv->enable_gpio, 0);
🤖 Fix all issues with AI agents
In `@config/boards/ayn-odin2-grub.csc`:
- Around line 55-61: The install lines set executable scripts with mode 655
(owner missing execute) which is incorrect; update the permission mode from 655
to 755 for all executable installs (the lines installing
setup-usbgadget-network.sh, remove-usbgadget-network.sh,
usb-gadget-initramfs-hook, usb-gadget-initramfs-premount, dropbear, and
kill-dropbear) while leaving the usbgadget-rndis.service install at 644; ensure
each install invocation for those script filenames uses 755 so the owner can
execute them.
In `@config/boards/ayn-odin2-portal.csc`:
- Around line 84-92: The mv calls that toggle armbian.sources (using
"${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled and armbian.sources)
can fail under errexit if the source/target file doesn't exist; update the block
around the chroot_sdcard_apt_get_update/install calls to guard the moves by
checking existence (e.g., test -e on
"${SDCARD}"/etc/apt/sources.list.d/armbian.sources.disabled before the first mv
and test -e on armbian.sources before the second mv) or make the moves no-op on
missing files (e.g., conditional mv or append || true) so
chroot_sdcard_apt_get_update and do_with_retries calls are not aborted.
In `@extensions/odin2-preset-firstrun.sh`:
- Around line 60-74: The script currently clones the repo twice (chroot_sdcard
git clone ... to /usr/local/odin2-scripts and the runtime installer written to
${launcher_file} which clones to ~/sys/odin2-scripts) causing redundancy; decide
one of two fixes: (A) If scripts must be preinstalled, remove the git clone
inside the installer and change the installer contents to reference
/usr/local/odin2-scripts (update references to ${launcher_dir}/${launcher_file}
or install-odin2-scripts accordingly), or (B) If scripts must be installed on
first run, remove the build-time chroot_sdcard git clone line and keep the
installer as the sole cloner; also remove or change the final cd
~/sys/odin2-scripts in the installer since it doesn’t affect the calling
shell—either document that the installer must be sourced to change the caller’s
directory or replace it with an explicit command that runs the intended setup
actions from that directory.
- Around line 41-48: The script currently writes weak static passwords into
.not_logged_in_yet (PRESET_ROOT_PASSWORD, PRESET_USER_PASSWORD,
PRESET_USER_NAME); update the preset logic so passwords are not hardcoded to
"1234" — either write empty values (e.g. PRESET_ROOT_PASSWORD= and
PRESET_USER_PASSWORD=) to force creation at first login, or generate and store
strong random passwords if the first-run flow supports it; ensure the entries
still use the same keys (PRESET_ROOT_PASSWORD, PRESET_USER_PASSWORD,
PRESET_USER_NAME) so downstream first-run code can read them.
In `@patch/kernel/archive/sm8550-6.13/0000.patching_config.yaml`:
- Line 1: The file-level comment at the top incorrectly references sm8250-6.7;
update the comment string in the header (the line starting with "config: # This
is file") to reference the correct path "sm8550-6.13/0000.patching_config.yaml"
so the inline comment matches the actual file location.
In
`@patch/kernel/archive/sm8550-6.13/0003_media--iris--implement-iris-v4l2-file-ops.patch`:
- Around line 331-354: The vb2 queues in iris_m2m_queue_init are missing
required fields before calling vb2_queue_init: set src_vq->ops and dst_vq->ops
to the instance's vb2 ops (inst->core->iris_vb2_ops), set src_vq->mem_ops and
dst_vq->mem_ops to &vb2_dma_contig_memops, and set src_vq->buf_struct_size and
dst_vq->buf_struct_size to sizeof(struct iris_buffer); ensure these assignments
occur for both src_vq and dst_vq (alongside the existing
type/io_modes/timestamp_flags/drv_priv/dev/lock) before calling vb2_queue_init
on each queue.
In
`@patch/kernel/archive/sm8550-6.13/0005_media--iris--implement-video-firmware-load-unload.patch`:
- Around line 147-149: The memremap failure path in iris_load_fw_to_memory does
a goto err_release_fw without setting ret, so a previous success leaves ret==0
and a failure is reported as success; update the memremap error handling (the
block that calls memremap(mem_phys, res_size, MEMREMAP_WC) and tests mem_virt)
to set ret to an appropriate negative error code (e.g., -ENOMEM or -EIO) before
jumping to err_release_fw, and ensure iris_load_fw_to_memory returns that error
instead of 0.
In
`@patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch`:
- Around line 677-690: The function iris_enable_power_domains incorrectly
returns the positive usage count from pm_runtime_get_sync (in
iris_enable_power_domains), causing callers to treat success as error; change
the logic so after calling pm_runtime_get_sync(pd_dev) you check for ret < 0 and
return that error, but on non-negative success return 0 (i.e., normalize any
non-error pm_runtime_get_sync result to 0) while preserving the existing
dev_pm_opp_set_rate(core->dev, ULONG_MAX) error handling.
In
`@patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch`:
- Around line 1462-1477: The helper iris_add_session currently silently skips
adding when core->iris_platform_data->max_session_count is reached; change its
signature to return an int (e.g., 0 on success, -EUSERS or -ENOSPC when full),
keep the mutex_lock/mutex_unlock and count logic but return an error instead of
dropping silently, and update the caller iris_open to check the return value and
propagate the error to the caller (cleaning up any partially initialized state
if needed) so the caller knows the session wasn't tracked; reference
functions/fields: iris_add_session, iris_open,
core->iris_platform_data->max_session_count, list_add_tail, core->instances,
mutex_lock/mutex_unlock.
In
`@patch/kernel/archive/sm8550-6.13/0015_media--iris--implement-query_cap-ioctl.patch`:
- Around line 29-35: The current iris_querycap implementation only fills driver
and card, leaving cap->capabilities and cap->device_caps zero; update
iris_querycap(struct file *filp, void *fh, struct v4l2_capability *cap) to set
cap->capabilities to include V4L2_CAP_DEVICE_CAPS (and any other global caps you
support), populate cap->device_caps with the appropriate device-level flags for
a video M2M decoder (e.g., V4L2_CAP_VIDEO_M2M_MPLANE and V4L2_CAP_STREAMING as
applicable), and fill cap->bus_info with a unique bus identifier (for example
using strscpy from the underlying device name or pci/dev identifier). Ensure you
reference IRIS_DRV_NAME and v4l2_capability fields when making these changes so
userspace can discover the device capabilities.
In
`@patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch`:
- Around line 268-286: The loop in iris_set_properties currently ignores the
return value of each per-capability setter, so failures from cap->set() are
swallowed; update iris_set_properties to capture the int ret = cap->set(inst, i)
for each valid capability (after checking iris_valid_cap_id and cap->cap_id) and
if ret is non-zero immediately return that error (propagating failures), while
keeping the existing early return for hfi_ops->session_set_config_params;
reference functions/fields: iris_set_properties,
hfi_ops->session_set_config_params, iris_valid_cap_id, inst->fw_caps, and
cap->set.
In
`@patch/kernel/archive/sm8550-6.13/0018_media--iris--subscribe-parameters-and-properties-t.patch`:
- Around line 87-227: The functions iris_hfi_gen2_subscribe_change_param and
iris_hfi_gen2_subscribe_property write into a fixed payload[32] without bounds
checks and continue on unsupported properties; add explicit checks that
change_param_size + 1 and subscribe_prop_size + 1 do not exceed
ARRAY_SIZE(payload) and return -EINVAL immediately if they do, and when the
default case in the change_param switch is hit return the error immediately (do
not continue the loop or set
inst_hfi_gen2->opsc_properties_set/ipsc_properties_set), and ensure the payload
pointer passed to
iris_hfi_gen2_session_set_property/iris_hfi_gen2_session_subscribe_mode uses the
payload buffer correctly (e.g., &payload[0]) after the bounds check.
In
`@patch/kernel/archive/sm8550-6.13/0021_media--iris--add-support-for-dynamic-resolution-ch.patch`:
- Around line 61-62: The switch in iris_hfi_get_v4l2_color_primaries has the
DCI-P3 branch reversed (it currently matches V4L2_COLORSPACE_DCI_P3 and returns
HFI_PRIMARIES_SMPTE_RP431_2); change that branch to match the HFI constant and
return the V4L2 constant instead—i.e., use case HFI_PRIMARIES_SMPTE_RP431_2:
return V4L2_COLORSPACE_DCI_P3; so the function consistently maps HFI primaries
-> V4L2 values.
In
`@patch/kernel/archive/sm8550-6.13/0023_media--iris--add-support-for-drain-sequence.patch`:
- Around line 103-134: The stack-local flush_pkt in
iris_hfi_gen1_session_ftb_done is only partially populated and may contain
uninitialized garbage; zero-initialize flush_pkt before setting its
header/session/flush_type (e.g., use memset or an aggregate initializer) so all
fields are deterministic, then keep the existing assignments to
flush_pkt.shdr.hdr.size, pkt_type, shdr.session_id, and flush_type and call
iris_hfi_queue_cmd_write, reinit_completion(&inst->flush_completion) and
iris_inst_sub_state_change_drain_last as present.
🧹 Nitpick comments (16)
patch/kernel/archive/sm8550-6.13/0020_media--iris--implement-vb2-ops-for-buf_queue-and-f.patch (2)
489-507: Minor: Dead code check onbufferspointer.The check
if (!buffers)at line 491 (patch context) is dead code sincebuffersis assigned from&inst->buffers[BUF_DPB], which is a fixed array member ofstruct iris_instand can never be NULL.However, this is upstream code that has been reviewed and merged, and the check is defensive/harmless. No action required unless you want to submit a cleanup patch upstream.
728-757: Minor: Unusedretvariable iniris_hfi_gen2_handle_session_info.The variable
retis initialized to 0 and returned unchanged. This could be simplified, but since this is upstream code, it may be intentional for future extension or consistency with other handlers.int ret = 0; // ... switch statement that doesn't modify ret ... return ret;No action needed - upstream reviewers accepted this pattern.
patch/kernel/archive/sm8550-6.13/0039_phy--qcom--qmp-combo--introduce-QPHY_MODE.patch (1)
50-75: Add a defensive default for unexpected init_mode values.If
init_modeever ends up with an invalid value, the switch will skip all resets and mode programming silently. A fallback to COMBO (optionally with a warn) keeps behavior safe.Proposed tweak
- switch (qmp->init_mode) { - case QPHY_MODE_COMBO: + switch (qmp->init_mode) { + case QPHY_MODE_COMBO: + default: + if (qmp->init_mode != QPHY_MODE_COMBO) + WARN_ONCE(1, "qmp-combo: unknown init_mode %d, defaulting to COMBO\n", + qmp->init_mode); writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL); /* bring both QMP USB and QMP DP PHYs PCS block out of reset */ qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL, SW_DPPHY_RESET_MUX | SW_DPPHY_RESET | SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET); break;patch/kernel/archive/sm8550-6.13/0021_media--iris--add-support-for-dynamic-resolution-ch.patch (2)
304-369: Consider validating data pointer bounds during property parsing.The loop iterates based on
num_properties_changedfrom the firmware packet, advancingdata_ptrthrough the packet data. If the firmware sends malformed data wherenum_properties_changedexceeds the actual properties present, or if individual property sizes are incorrect, this could potentially read beyond the packet buffer.Given this is upstream kernel code that has been reviewed and the firmware is trusted, this may be acceptable. However, adding bounds checking (e.g., tracking remaining bytes and validating before each read) would make this more robust against firmware bugs.
622-627: Potential unsigned underflow in crop dimension calculation.The crop width/height calculations subtract offsets from dimensions without validation. If firmware provides crop offsets larger than the image dimensions (e.g., due to a firmware bug), the subtraction would underflow since these are unsigned values, resulting in very large crop dimensions.
Since this is upstream kernel code with trusted firmware interaction, this may be acceptable. The code at lines 633-638 does check for unsupported content and transitions to error state, which provides some protection.
patch/kernel/archive/sm8550-6.13/0024_media--iris--add-check-whether-the-video-session-i.patch (1)
108-142: Locking inconsistency in instance list iteration.The function iterates
core->instanceswithout holdingcore->lock(line 115-118), whileiris_check_core_mbpf()acquires the lock before iterating the same list. This creates a potential race window if instances are added/removed concurrently.For consistency and safety, consider holding
core->lockduring the instance verification check iniris_check_session_supported(), matching the locking discipline used iniris_check_core_mbpf().Note: This is upstream kernel code that has passed multiple rounds of review and been tested on multiple hardware platforms. The inconsistency may be intentional if callers are expected to hold the lock, but harmonizing the locking would improve safety.
extensions/odin2-preset-firstrun.sh (1)
57-65: Variablelauncher_diris shadowed with a different value.
launcher_diris assigned on line 57 (${SDCARD}/usr/local) and then reassigned on line 63 (${SDCARD}/usr/local/bin). While this works, it reduces readability. Use distinct variable names for clarity.Proposed fix
- local launcher_dir="${SDCARD}/usr/local" - run_host_command_logged mkdir -pv "${launcher_dir}" + local scripts_dir="${SDCARD}/usr/local" + run_host_command_logged mkdir -pv "${scripts_dir}" - chroot_sdcard git clone https://github.com/Squishy123/odin2-scripts.git "$launcher_dir/odin2-scripts" + chroot_sdcard git clone https://github.com/Squishy123/odin2-scripts.git "${scripts_dir}/odin2-scripts" local launcher_dir="${SDCARD}/usr/local/bin"patch/kernel/archive/sm8550-6.13/0025_media--iris--implement-power-scaling-for-vpu2-and-.patch (1)
258-270: Hardcoded DEFAULT_FPS (30) for bandwidth voting.The function uses
DEFAULT_FPSinstead of the actual frame rate, which may not accurately reflect real bandwidth requirements for 60fps or variable frame rate content. This appears to be an intentional simplification in the initial implementation.config/desktop/trixie/environments/kde-plasma-mobile/config_base/packages (1)
12-20: For full PipeWire audio on KDE Plasma Mobile, addpipewire-pulseandwireplumber.Adding
pipewirealone is incomplete for Debian Trixie. KDE Plasma's audio controls use the PulseAudio API, so you needpipewire-pulse(the PulseAudio-compatible server) not justpipewire, pluswireplumberas the session manager. Thepulseaudiopackages won't cause hard conflicts withpipewirein Trixie, but only one sound daemon should be active per session—ensurepipewire-pulseis configured to start instead ofpulseaudio. Also verify thatplasma-paremains installed so the audio applet doesn't disappear.If this is only for camera/screencast features, then
pipewirealone is acceptable; please confirm the intended audio stack.packages/bsp/ayn-odin2portal/zz-update-abl-kernel (1)
4-11: Glob patterns may match multiple files unexpectedly.The glob patterns (
vmlinuz-*-sm8550-arm64,linux-image-*-sm8550-arm64,initrd.img-*-sm8550-arm64) could match multiple files if more than one kernel version is installed, leading to incorrect boot image generation:
- Line 4:
gzip -c /boot/vmlinuz-*would concatenate multiple kernels- Line 11:
--ramdisk /boot/initrd.img-*would fail or use wrong fileAlso, Line 8 sources
/boot/armbianEnv.txtbut no variables from it appear to be used in this script.Consider using more specific matching or validating single matches:
♻️ Suggested improvement
#!/bin/bash set -ex new_rootfs_image_uuid=$(sed -e 's/^.*root=UUID=//' -e 's/ .*$//' < /proc/cmdline) -gzip -c /boot/vmlinuz-*-sm8550-arm64 > /tmp/Image.gz + +# Find the latest installed kernel +kernel_img=$(ls -1t /boot/vmlinuz-*-sm8550-arm64 2>/dev/null | head -1) +[[ -z "$kernel_img" ]] && { echo "No kernel found"; exit 1; } +kernel_version="${kernel_img#/boot/vmlinuz-}" + +gzip -c "$kernel_img" > /tmp/Image.gz -cat /tmp/Image.gz /usr/lib/linux-image-*-sm8550-arm64/qcom/qcs8550-ayn-odin2-hypdtbo.dtb > /tmp/Image.gz-dtb +cat /tmp/Image.gz "/usr/lib/linux-image-${kernel_version}/qcom/qcs8550-ayn-odin2-hypdtbo.dtb" > /tmp/Image.gz-dtb -source /boot/armbianEnv.txt /usr/bin/mkbootimg \ --kernel /tmp/Image.gz-dtb \ - --ramdisk /boot/initrd.img-*-sm8550-arm64 \ + --ramdisk "/boot/initrd.img-${kernel_version}" \config/boards/ayn-odin2-portal-grub.csc (1)
1-1: Align the header comment with the standard single-line hardware spec format.Consider updating the header to the usual one-line hardware summary (SoC, RAM options, key features) to match other board configs.
Based on learnings: In Armbian board configuration files, the standard pattern is to have only one line as a comment describing the board hardware specifications.
patch/kernel/archive/sm8550-6.13/0000.patching_config.yaml (1)
3-8: Remove unused metadata fields.These metadata fields (
name,kind,type,branch,last-known-good-tag) are not used by the patching scripts and should be removed entirely. Based on learnings from this repository, these fields serve no functional purpose.Proposed fix
config: # This is file 'patch/kernel/archive/sm8550-6.13/0000.patching_config.yaml' - # Just some info stuff; not used by the patching scripts - name: sm8550-6.13 - kind: kernel - type: mainline # or: vendor - branch: linux-6.13.y - last-known-good-tag: v6.13.11 - # .dts files in these directories will be copied as-is to the build tree; later ones overwrite earlier ones.patch/kernel/archive/sm8550-6.13/0005_media--iris--implement-video-firmware-load-unload.patch (1)
182-185: Error code override loses specific failure reason.When
iris_load_fw_to_memoryfails, the specific error code is discarded and replaced with-ENOMEM. Consider propagating the actual error.Proposed fix
ret = iris_load_fw_to_memory(core, fwpath); if (ret) { dev_err(core->dev, "firmware download failed\n"); - return -ENOMEM; + return ret; }patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch (1)
634-653: State-change log prints the new state twice.
dev_dbg()logsinst->stateafter assignment, so “from” and “to” are identical. Capture the previous state before overwriting.🧭 Suggested fix
-change_state: - inst->state = request_state; - dev_dbg(inst->core->dev, "state changed from %x to %x\n", - inst->state, request_state); +change_state: + { + enum iris_inst_state prev_state = inst->state; + inst->state = request_state; + dev_dbg(inst->core->dev, "state changed from %x to %x\n", + prev_state, request_state); + }patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch (1)
503-507: Consider adding a named constant for the magic register offset.The register offset
0xB0088is a magic number. A named constant would improve readability and maintainability.patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch (1)
713-715: Consider using a named constant for packet allocation size.The magic number
4096for packet allocation could be replaced with a named constant for clarity.
patch/kernel/archive/sm8550-6.13/0015_media--iris--implement-query_cap-ioctl.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0018_media--iris--subscribe-parameters-and-properties-t.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0021_media--iris--add-support-for-dynamic-resolution-ch.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0023_media--iris--add-support-for-drain-sequence.patch
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
config/desktop/trixie/environments/kde-plasma-mobile/config_base/packages (1)
12-20:⚠️ Potential issue | 🟠 MajorPotential conflict:
pipewireadded alongsidepulseaudio.Installing both
pipewireandpulseaudio(lines 12, 18) can cause audio device contention at runtime. In Debian Trixie, the typical approach is one of:
- PipeWire as full replacement: Drop
pulseaudioandpulseaudio-module-bluetooth, addpipewire-pulse,pipewire-audio, andwireplumber. The PulseAudio client tools (pavucontrol, etc.) will continue to work via the compatibility layer.- PipeWire for video/portal only: If
pipewireis here solely for screen-sharing (xdg-desktop-portal), this is fine, but it would be worth adding a comment to clarify intent and avoid future confusion.Could you clarify the intended audio stack? If this device should use PipeWire for audio (which is the modern default for Plasma Mobile), the PulseAudio packages should be swapped out.
patch/kernel/archive/sm8550-6.13/0036_ASoC--qcom--sc8280xp-Add-support-for-Primary-I2S.patch (1)
35-55:⚠️ Potential issue | 🟡 MinorUnchecked return value from
clk_prepare_enable.
clk_prepare_enable()can fail, and its return value should be checked and propagated. Currently the startup function ignores the error and continues.Proposed fix
+static int sc8280xp_snd_startup(struct snd_pcm_substream *substream) +{ + ... + struct sc8280xp_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card); + ... + switch (cpu_dai->id) { + case PRIMARY_MI2S_RX: - clk_prepare_enable(pdata->i2s_clk); + ret = clk_prepare_enable(pdata->i2s_clk); + if (ret) + return ret; snd_soc_dai_set_fmt(cpu_dai, fmt); snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt); break;patch/kernel/archive/sm8550-6.13/0032_drm-panel--Add-panel-driver-for-Chipone-ICNA3512-b.patch (2)
263-300:⚠️ Potential issue | 🟠 MajorResource leak on error paths in
icna3512_prepare.After
init_sequencesucceeds, ifmipi_dsi_picture_parameter_set(line 286) ormipi_dsi_compression_mode(line 293) fails, the function returns without disabling regulators or de-asserting the reset GPIO. The cleanup at line 279 only covers theinit_sequencefailure path.Proposed fix — add cleanup to remaining error paths
ret = mipi_dsi_picture_parameter_set(pinfo->dsi, &pps); if (ret < 0) { dev_err(panel->dev, "failed to transmit PPS: %d\n", ret); - return ret; + goto err_disable; } /* Not required, ICNA3512 has DSC always enabled. */ ret = mipi_dsi_compression_mode(pinfo->dsi, true); if (ret < 0) { dev_err(panel->dev, "failed to enable compression mode: %d\n", ret); - return ret; + goto err_disable; } return 0; + +err_disable: + regulator_bulk_disable(ARRAY_SIZE(pinfo->supplies), pinfo->supplies); + return ret; }
431-442:⚠️ Potential issue | 🟡 MinorBacklight default brightness equals max brightness.
Both
brightnessandmax_brightnessare set to 4096. This means the panel starts at maximum brightness. Typicallybrightnessis set lower (e.g., 2/3 of max) to avoid blinding the user on first power-on and to leave headroom for brightness adjustments.patch/kernel/archive/sm8550-6.13/0033_leds--Add-driver-for-HEROIC-HTR3212.patch (2)
292-311:⚠️ Potential issue | 🟠 MajorRegulator enabled but never disabled on probe error paths.
regulator_enable(priv->vdd)at line 292 is not matched byregulator_disableifhtr3212_init_regs(line 303) orhtr3212_parse_dt(line 307) fails. The regulator was obtained viadevm_regulator_getwhich only manages theget/putlifecycle, not enable/disable. This leaks a regulator enable reference.Proposed fix — add cleanup on error
ret = htr3212_init_regs(priv); - if (ret) - return ret; + if (ret) { + regulator_disable(priv->vdd); + return ret; + } ret = htr3212_parse_dt(dev, priv); - if (ret) - return ret; + if (ret) { + regulator_disable(priv->vdd); + return ret; + }
348-348:⚠️ Potential issue | 🟡 MinorMissing closing
>inMODULE_AUTHOR.The email address is missing the closing angle bracket.
Proposed fix
-MODULE_AUTHOR("Teguh Sobirin <teguh@sobir.in"); +MODULE_AUTHOR("Teguh Sobirin <teguh@sobir.in>");patch/kernel/archive/sm8550-6.13/0031_input--Add-driver-for-RSInput-Gamepad.patch (6)
54-54:⚠️ Potential issue | 🟠 Major
#define DEBUGleft in — enables alldev_dbgcalls unconditionally.This should be removed before merging. It was likely left in during development and will produce excessive kernel log output in production.
Proposed fix
-#define DEBUG - `#include` <linux/errno.h>
281-314:⚠️ Potential issue | 🔴 CriticalSleeping in atomic/callback context —
rsinput_init_commandscalled fromrsinput_rx.
rsinput_rxis the serdev.receive_bufcallback, invoked from the serial core, potentially in interrupt or softirq context. On checksum mismatch (line 307), it callsrsinput_init_commandswhich containsmsleep(100)— sleeping in a non-sleepable context will trigger a BUG.This re-initialization on bad checksum should be deferred to a workqueue.
333-352:⚠️ Potential issue | 🔴 CriticalGPIO error pointers stored and later dereferenced.
devm_gpiod_get_optionalreturnsERR_PTRon real failures. The code warns but does not return or clear the error pointer. Later,if (drv->boot_gpio)/if (drv->reset_gpio)/if (drv->enable_gpio)are truthy forERR_PTRvalues, sogpiod_set_value_cansleepwill be called on an invalid pointer (lines 354-367).Proposed fix — propagate errors
drv->boot_gpio = devm_gpiod_get_optional(&serdev->dev, "boot", GPIOD_OUT_HIGH); - if (IS_ERR(drv->boot_gpio)) { - error = PTR_ERR(drv->boot_gpio); - dev_warn(&serdev->dev, "Unable to get boot gpio: %d\n", error); - } + if (IS_ERR(drv->boot_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->boot_gpio), + "Unable to get boot gpio\n"); drv->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_HIGH); - if (IS_ERR(drv->enable_gpio)) { - error = PTR_ERR(drv->enable_gpio); - dev_warn(&serdev->dev, "Unable to get enable gpio: %d\n", error); - } + if (IS_ERR(drv->enable_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->enable_gpio), + "Unable to get enable gpio\n"); drv->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(drv->reset_gpio)) { - error = PTR_ERR(drv->reset_gpio); - dev_warn(&serdev->dev, "Unable to get reset gpio: %d\n", error); - } + if (IS_ERR(drv->reset_gpio)) + return dev_err_probe(&serdev->dev, PTR_ERR(drv->reset_gpio), + "Unable to get reset gpio\n");
370-428:⚠️ Potential issue | 🟠 Major
serdev_device_opennot closed on multiple error paths.After
serdev_device_opensucceeds at line 370, the following error paths return without callingserdev_device_close: baudrate failure (line 381), input allocation failure (line 387), and input registration failure (line 420). Only theinit_commandsfailure path (line 426) properly closes.Proposed fix — close serdev on all error paths
error = serdev_device_set_baudrate(serdev, 115200); if (error < 0) - return dev_err_probe(&serdev->dev, error, "Failed to set up host baud rate\n"); + goto err_close; serdev_device_set_flow_control(serdev, false); drv->input = devm_input_allocate_device(&serdev->dev); - if (!drv->input) - return -ENOMEM; + if (!drv->input) { + error = -ENOMEM; + goto err_close; + } ... error = input_register_device(drv->input); if (error) - return error; + goto err_close; ... + return 0; + +err_close: + serdev_device_close(serdev); + return error;
433-443:⚠️ Potential issue | 🔴 CriticalDouble-unregister of devm-managed input device.
drv->inputis allocated viadevm_input_allocate_device(line 385), which automatically handles unregistration on device removal. Callinginput_unregister_devicemanually inrsinput_remove(line 437) will cause a double-free/use-after-free.Proposed fix
static void rsinput_remove(struct serdev_device *serdev) { struct rsinput_driver *drv = serdev_device_get_drvdata(serdev); serdev_device_close(serdev); - input_unregister_device(drv->input); if (drv->enable_gpio) gpiod_set_value_cansleep(drv->enable_gpio, 0);
209-241:⚠️ Potential issue | 🟠 Major
staticlocalprev_statesis shared across all device instances.
prev_statesat line 211 is a function-localstatic, meaning it is shared if multiple RSInput devices are probed. It should be a field instruct rsinput_driver.Proposed fix
Add
unsigned long prev_states;tostruct rsinput_driverand referencedrv->prev_statesinstead:struct rsinput_driver { ... + unsigned long prev_states; };static void handle_cmd_status(...) { if (payload_length >= 6) { - static unsigned long prev_states; unsigned long keys = data[FRAME_POS_DATA_1] | (data[FRAME_POS_DATA_2] << 8); - unsigned long current_states = keys, changes; + unsigned long current_states = keys; + unsigned long changes; ... - prev_states = keys; + drv->prev_states = keys; }
🤖 Fix all issues with AI agents
In `@config/boards/ayn-odin2-grub.csc`:
- Around line 83-87: The trixie Mesa driver block is commented out causing GRUB
builds to skip installing Mesa Vulkan drivers; either restore the block or
document why it's intentionally disabled: if no GRUB+trixie issue exists,
uncomment the RELEASE-check block that calls do_with_retries with
chroot_sdcard_apt_get_update, display_alert "Installing Mesa Vulkan Drivers",
and do_with_retries chroot_sdcard_apt_get_install for libgl1-mesa-dri
mesa-vulkan-drivers vulkan-tools; if it is intentional, add a brief comment
above the RELEASE check explaining the GRUB+trixie incompatibility and reference
this behavior so maintainers know it’s deliberate.
In `@config/boards/ayn-odin2-portal-grub.csc`:
- Around line 38-45: The curl invocation that writes "temp.zip" should include
--fail (e.g. curl -L --fail -o temp.zip) and check its exit status before
running unzip so HTTP errors (404/etc.) don't produce invalid zip files; after
the curl command, if it fails, exit with a non-zero code and avoid calling unzip
on "temp.zip" (also remove any partial temp.zip). Additionally, replace the
branch zip URL with a pinned commit hash or tag instead of the branch path
("ayn/v1.2.13.zip") to ensure reproducible builds; update the code that
references temp.zip and unzip_dir accordingly.
- Around line 52-61: Fix the incorrect file mode and unquoted variables: change
all install mode arguments from 655 to 755 for the script/binary installs (the
lines installing setup-usbgadget-network.sh, remove-usbgadget-network.sh,
usb-gadget-initramfs-hook, usb-gadget-initramfs-premount, dropbear,
kill-dropbear and any other script installs) so owner execute bit is set, and
quote $destination and $SRC in the mkdir and install commands (e.g., use
"$destination" and "$SRC") to prevent word-splitting/expansion issues; keep the
same target paths and modes otherwise.
In `@config/kernel/old-linux-sm8550-edge.config`:
- Around line 934-936: The config enables AppArmor via
CONFIG_SECURITY_APPARMOR=y but does not include "apparmor" in the CONFIG_LSM
string; update CONFIG_LSM to include apparmor (e.g., add apparmor into the
comma-separated list used by CONFIG_LSM) so the LSM list contains "apparmor" and
keep CONFIG_SECURITY_APPARMOR=y in sync; ensure you don't duplicate entries and
preserve the existing order/commas in the CONFIG_LSM value.
- Line 1: The config file named old-linux-sm8550-edge.config does not follow the
Armbian naming convention and will be ignored; either delete
old-linux-sm8550-edge.config if it's obsolete, or rename it to
linux-sm8550-edge.config so the build system recognizes it, and ensure there are
no duplicate linux-sm8550-edge.config files (update any references to the old
name if present).
In
`@patch/kernel/archive/sm8550-6.13/0004_media--iris--introduce-iris-core-state-management-.patch`:
- Around line 519-530: The probe path allocates and calls mutex_init but on
failure returns via the err_vdev_unreg and err_v4l2_unreg labels without calling
mutex_destroy; update the probe failure cleanup to mirror iris_remove by calling
mutex_destroy(&core->lock) before those error labels return. Specifically, in
the probe function (after mutex_init) add mutex_destroy(&core->lock) in the
err_vdev_unreg and err_v4l2_unreg cleanup paths so the mutex is always destroyed
on probe failure; ensure you keep existing calls to iris_core_deinit,
video_unregister_device, and v4l2_device_unregister in their respective error
paths intact.
In
`@patch/kernel/archive/sm8550-6.13/0005_media--iris--implement-video-firmware-load-unload.patch`:
- Around line 182-184: The code discards the actual error from
iris_load_fw_to_memory by logging a generic message and returning -ENOMEM;
change the handler in the caller (where ret holds iris_load_fw_to_memory's
result) to log the real error and propagate it: use dev_err(core->dev, "firmware
download failed: %d\n", ret) (or similar to include error text) and return ret
instead of returning -ENOMEM so callers receive the original errno from
iris_load_fw_to_memory.
- Around line 156-167: The success path currently returns immediately after
calling qcom_scm_pas_auth_and_reset, leaking mem_virt and firmware; fix by
performing the same cleanup as the error paths before returning on success:
after qcom_scm_pas_auth_and_reset succeeds, call memunmap(mem_virt) and
release_firmware(firmware) (or reorder labels so the normal return jumps to a
cleanup label that calls memunmap and release_firmware) then return ret; keep
the existing err_mem_unmap/err_release_fw labels for error flows and ensure
mem_virt and firmware are only released once.
In
`@patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch`:
- Around line 421-446: The early-return path in iris_hfi_queue_cmd_write wrongly
calls pm_runtime_put_sync even when pm_runtime_resume_and_get failed, causing a
usage-count underflow; update the control flow so that after calling
pm_runtime_resume_and_get (in iris_hfi_queue_cmd_write) you only call
pm_runtime_put_sync if pm_runtime_resume_and_get succeeded (i.e., when you later
need to unwind after a failure from iris_hfi_queue_cmd_write_locked), while
retaining the current mutex_lock/ mutex_unlock and the pm_runtime_mark_last_busy
+ pm_runtime_put_autosuspend sequence on the success path; in practice, split
the exit handling into two paths (one for resume failure that skips
pm_runtime_put_sync, and one for post-resume errors that calls
pm_runtime_put_sync before returning).
- Around line 1174-1177: The call to dev_pm_opp_set_rate(core->dev, freq) must
have its return value checked and handled; replace the discarded-call with code
that captures the return (e.g., ret = dev_pm_opp_set_rate(...)) and if ret < 0
jumps to an err_power_off_hw label that tears down the hardware power domain
(mirroring the cleanup done in iris_enable_power_domains failure paths) and then
falls through to the existing err_power_off_ctrl label to finish cleanup; add
the err_power_off_hw label if missing and ensure it undoes the HW power domain
enable before proceeding to err_power_off_ctrl.
In
`@patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch`:
- Around line 1009-1047: The handler loop in
iris_hfi_gen2_handle_session_response currently overwrites ret for each packet
so earlier errors are lost; after calling range[i].handle(inst, packet) store
its return into a temporary (or into ret) and if it is non-zero immediately
break out of the loops (or return) so the first error is propagated; ensure you
still unlock inst->lock before returning (i.e., check the handler result, break
both inner/outer loops or goto out to run mutex_unlock(&inst->lock) and then
return ret); reference the variables/funcs:
iris_hfi_gen2_handle_session_response, ret, range[i].handle, packet, inst->lock
and hdr->num_packets when making the change.
- Around line 1233-1247: iris_get_instance currently returns a raw pointer after
dropping core->lock which exposes a TOCTOU/use-after-free race (e.g., between
iris_get_instance and iris_close or callers like
iris_hfi_gen1_handle_response/iris_hfi_gen2_handle_session_response). Fix by
ensuring the instance is held before releasing core->lock: either (A) acquire
inst->lock while still holding core->lock (grab core->lock, find inst,
mutex_lock(&inst->lock), then mutex_unlock(&core->lock) and return inst) or (B)
add kref-based refcounting to struct iris_inst and call kref_get(&inst->ref)
while still holding core->lock and drop core->lock before returning, then
require callers to call kref_put when done; update iris_close to use kref_put
and to remove/free only when refcount reaches zero. Ensure all callers follow
the new refcount/lock semantics.
- Around line 1031-1041: The packet-level HFI_FW_FLAGS_SESSION_ERROR check
currently coexists with the range dispatch and causes
iris_hfi_gen2_handle_session_error to be invoked twice for packets whose type
falls within the HFI_SESSION_ERROR range; update the logic so that when
(packet->flags & HFI_FW_FLAGS_SESSION_ERROR) is true you call
iris_hfi_gen2_handle_session_error(inst, packet) and then skip the range handler
(e.g., continue to next packet) instead of letting the subsequent
range[i].handle call run, or alternatively move the flag check outside the for
(i ...) range loop and only run it once per packet before any range dispatches.
- Around line 57-81: The iris_vb2_ops struct is missing wait callbacks which
causes ctx_q_lock (assigned to vb2_queue.lock) to be held while VB2 blocks,
leading to deadlocks; update the iris_vb2_ops definition to add .wait_prepare =
vb2_ops_wait_prepare and .wait_finish = vb2_ops_wait_finish so the queue lock
(vb2_queue.lock / ctx_q_lock) is released during blocking operations like DQBUF
and properly re-acquired afterwards.
In
`@patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch`:
- Around line 648-654: The debug log prints the same value because inst->state
is assigned before logging; in the change_state block capture the previous state
into a temporary (e.g., old_state) before assigning inst->state = request_state,
then call dev_dbg(inst->core->dev, "state changed from %x to %x\n", old_state,
request_state); reference the change_state label and the
inst->state/request_state variables and update the dev_dbg argument order to use
the saved old_state.
In
`@patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch`:
- Line 1055: Rename the misspelled function iris_hfi_gen2_set_bit_dpeth to
iris_hfi_gen2_set_bit_depth and update all references (including the entry in
the handler table that currently points to iris_hfi_gen2_set_bit_dpeth) to use
the corrected symbol; ensure the function prototype, definition, and any forward
declarations match the new name so linkage and the handler table resolve
correctly.
- Around line 798-823: The code always loads
core->iris_platform_data->input_config_params into config_params and
config_params_size so the V4L2_TYPE_IS_CAPTURE branch compares capture plane
property IDs against prop_type_handle_out_arr using the input list (silently
skipping capture properties); fix by selecting the correct platform array based
on plane before the loops—i.e., set config_params/config_params_size =
core->iris_platform_data->input_config_params/input_config_params_size when
V4L2_TYPE_IS_OUTPUT(plane) and set them =
core->iris_platform_data->output_config_params/output_config_params_size when
V4L2_TYPE_IS_CAPTURE(plane), then keep the existing loops that match
config_params[i] against prop_type_handle_inp_arr or prop_type_handle_out_arr
and call their .handle(inst).
In
`@patch/kernel/archive/sm8550-6.13/0020_media--iris--implement-vb2-ops-for-buf_queue-and-f.patch`:
- Around line 488-506: The null-check on buffers is dead because buffers is
assigned from &inst->buffers[BUF_DPB], an embedded array element that cannot be
NULL; remove the if (!buffers) goto error; branch and any associated error label
reliance, and simply iterate/list_for_each_entry over
inst->buffers[BUF_DPB].list after assigning buffers = &inst->buffers[BUF_DPB];
(keep the surrounding logic: iris_split_mode_enabled(inst), pkt->stream_id == 0,
found detection comparing iter->index to output_tag and iter->data_offset to
offset, and setting buf = iter when found).
In
`@patch/kernel/archive/sm8550-6.13/0033_leds--Add-driver-for-HEROIC-HTR3212.patch`:
- Around line 282-284: The devm_gpiod_get call for priv->sdb must be checked
with IS_ERR before calling PTR_ERR; change the logic in the probe path where
devm_gpiod_get(dev, "sdb", GPIOD_OUT_HIGH) is assigned to first test if
(IS_ERR(priv->sdb)) then get the error via PTR_ERR(priv->sdb) and if that equals
-EPROBE_DEFER return -EPROBE_DEFER else return that error (or propagate
appropriately) so you never store an error pointer in priv->sdb that later gets
dereferenced by gpiod_set_value_cansleep.
🧹 Nitpick comments (17)
patch/kernel/archive/sm8550-6.13/0008_media--iris--implement-power-management.patch (1)
594-598:__maybe_unusedannotations should be dropped in favor ofpm_sleep_ptr()/DEFINE_RUNTIME_DEV_PM_OPS.The
SET_SYSTEM_SLEEP_PM_OPS/SET_RUNTIME_PM_OPSmacros together with__maybe_unusedare the legacy pattern. Modern kernels (≥ 6.4) preferDEFINE_RUNTIME_DEV_PM_OPSorpm_sleep_ptr()which let the compiler drop dead code without the__maybe_unusedworkaround. Since this targets 6.13, the newer macros are available.patch/kernel/archive/sm8550-6.13/0007_media--iris--introduce-host-firmware-interface-wit.patch (2)
625-638: Variable shadowing: innerpktshadows outerpktiniris_hfi_gen1_flush_debug_queue.Line 633 declares
struct hfi_msg_sys_debug_pkt *pktinside theifblock, shadowing the outerstruct hfi_msg_sys_coverage_pkt *pktat line 627. This is valid C and works correctly since the inner scope uses the debug packet type while the outer uses coverage, but it can confuse readers and some compilers will warn with-Wshadow. Since this is upstream Qualcomm code carried as a patch, this is a minor observation.
1569-1576:iris_sys_error_handlerre-inits without cancelling itself or checking state.If
iris_core_initat line 1575 fails (e.g., firmware load timeout), it may trigger another system error, which schedulessys_error_handleragain, creating an unbounded retry loop. There's no backoff, retry limit, or cancellation of the delayed work before re-init. Since this is upstream code, this is a minor observation — the 10ms delay provides some throttling, and theIRIS_CORE_ERRORstate gate iniris_wait_for_system_responsewould return-EIObefore re-triggering HFI init.patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch (1)
535-561: Silently swallowing-EOPNOTSUPPbut the callee never returns it.
hfi_gen1_set_propertyconverts-EOPNOTSUPPto success, butiris_hfi_gen1_packet_session_set_propertyreturns-EINVALfor unknown property types, never-EOPNOTSUPP. The special-case is dead code. Consider aligning the error codes or removing the-EOPNOTSUPPhandling.config/boards/ayn-odin2.csc (2)
69-82: Mesa driver handling restructured appropriately.The split between Ubuntu releases (using kisak-mesa PPA) and Debian trixie (using stock repos) is correct. However, note that the trixie block could be simplified by combining with the Ubuntu block's install lines since only the PPA addition differs.
114-119: Firmware glob onsm8550/*will match subdirectories too.The glob
/lib/firmware/qcom/sm8550/*will match both files and theayn/subdirectory itself. Whileadd_firmwarelikely handles directory entries gracefully, the loop at Line 117–119 forsm8550/ayn/*is partially redundant since files directly insm8550/ayn/would have their parent dir entry already matched. This is a minor nit — the behavior is correct sinceadd_firmwareshould skip non-files.config/boards/ayn-odin2-grub.csc (1)
1-141: Significant code duplication across board configs.This file is nearly identical to
ayn-odin2.csc(minus ABL-specific settings, plus GRUB settings). The same pattern repeats acrossayn-odin2-portal.cscand presumablyayn-odin2-portal-grub.csc. Functions likeis_userspace_supported,alsa_ucm_conf,firmware,enable_services, andbsp_firmware_in_initrdare copy-pasted with minimal differences.Consider extracting shared logic into a common include file (e.g.,
config/boards/include/ayn-odin2-common.inc) to reduce maintenance burden. Any bug fix (like the 655→755 permission fix) currently needs to be applied in 4 places.config/boards/ayn-odin2-portal-grub.csc (1)
74-87: Mesa driver installation logic is duplicated across release branches.The
apt_get_update+apt_get_installfor Mesa Vulkan drivers is identical for both the Ubuntu and Trixie paths. Consider extracting the common install block and only conditionally adding the PPA for Ubuntu releases.♻️ Suggested refactor
- if [[ "${RELEASE}" == "jammy" ]] || [[ "${RELEASE}" == "noble" ]] || [[ "${RELEASE}" == "plucky" ]]; then - display_alert "Adding Mesa PPA For Ubuntu ${BOARD}" "warn" - do_with_retries 3 chroot_sdcard add-apt-repository ppa:kisak/kisak-mesa --yes - - do_with_retries 3 chroot_sdcard_apt_get_update - display_alert "Installing Mesa Vulkan Drivers" - do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-tools - fi - - if [[ "${RELEASE}" == "trixie" ]]; then - do_with_retries 3 chroot_sdcard_apt_get_update - display_alert "Installing Mesa Vulkan Drivers" - do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-tools - fi + if [[ "${RELEASE}" == "jammy" ]] || [[ "${RELEASE}" == "noble" ]] || [[ "${RELEASE}" == "plucky" ]]; then + display_alert "Adding Mesa PPA For Ubuntu ${BOARD}" "warn" + do_with_retries 3 chroot_sdcard add-apt-repository ppa:kisak/kisak-mesa --yes + fi + + do_with_retries 3 chroot_sdcard_apt_get_update + display_alert "Installing Mesa Vulkan Drivers" + do_with_retries 3 chroot_sdcard_apt_get_install libgl1-mesa-dri mesa-vulkan-drivers vulkan-toolspatch/kernel/archive/sm8550-6.13/0032_drm-panel--Add-panel-driver-for-Chipone-ICNA3512-b.patch (1)
133-201: Inconsistent indentation — mixed tabs and spaces throughouticna3512_init_sequence.The function uses tabs for some
mipi_dsi_dcs_write_seqcalls and spaces for others (e.g., lines 151, 154, 157-160 vs. lines 145, 152). The} else {at line 162 also violates kernel coding style (should be on the same line as}). This pattern recurs throughout the file (lines 177-181, 189-191, 269, 279, 327, 483, 490-499).patch/kernel/archive/sm8550-6.13/0027_media--iris--enable-video-driver-probe-of-SM8250-S.patch (1)
212-217: Inconsistent indentation iniris_dt_matchtable entry.The SM8250 entry uses double-indented braces compared to the SM8550 entry above it. This should match the surrounding style for consistency.
Proposed fix
`#if` (!IS_ENABLED(CONFIG_VIDEO_QCOM_VENUS)) - { - .compatible = "qcom,sm8250-venus", - .data = &sm8250_data, - }, + { + .compatible = "qcom,sm8250-venus", + .data = &sm8250_data, + }, `#endif`patch/kernel/archive/sm8550-6.13/0024_media--iris--add-check-whether-the-video-session-i.patch (1)
108-142: Unprotectedcore->instanceslist traversal iniris_check_session_supported.The loop at the "found" check (lines 115-118 in the patch diff) iterates
core->instanceswithout holdingcore->lock, whileiris_check_core_mbpf(called immediately after) acquires the same lock for its own traversal. A concurrentiris_open/iris_closecould modify the list during the unprotected iteration.That said, this is an upstream-accepted patch (reviewed/tested by multiple Linaro engineers), and the caller likely already holds
inst->lockwhich may provide sufficient serialization in practice. Flagging for awareness rather than as a blocker.patch/kernel/archive/sm8550-6.13/0021_media--iris--add-support-for-dynamic-resolution-ch.patch (1)
304-369:defaultcase in property-parsing loop does not advancedata_ptr.In
iris_hfi_gen1_read_changed_params, when theswitch(ptype)hits thedefaultbranch,data_ptris not advanced. The loop will re-read the same unknownptypevalue on every remaining iteration (untilnum_properties_changeddecrements to zero), silently skipping all subsequent properties.This is upstream-accepted code, but worth noting: if the firmware ever sends an unrecognized property, all properties after it in the notification will be lost.
patch/kernel/archive/sm8550-6.13/0018_media--iris--subscribe-parameters-and-properties-t.patch (1)
99-103: Misleading error log and inconsistent return value when properties are already set.When
ipsc_properties_setoropsc_properties_setis alreadytrue, the code logs"invalid plane"(which is inaccurate—the plane is valid, the properties were just already submitted) and returns0(success) despite logging at error level. Consider either making this a no-op silently, or returning an error if it's truly unexpected.patch/kernel/archive/sm8550-6.13/0025_media--iris--implement-power-scaling-for-vpu2-and-.patch (1)
316-334:iris_scale_powerresume-and-put pattern has a subtle TOCTOU window.Between
pm_runtime_put_autosuspend(line 326) and the subsequentiris_scale_clocks→dev_pm_opp_set_ratecall, the device could autosuspend again. In practicedev_pm_opp_set_rateinternally handles runtime PM, so this is unlikely to cause issues. Just noting for awareness.patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch (1)
851-865: State checked without holdinginst->lock.Lines 858–859 read
inst->statebeforeinst->lockis acquired on line 861. The same pattern appears iniris_vb2_stop_streamingat lines 897–898. While VB2 typically serializes streaming ops per queue, a concurrent error-path or system-error handler (which does iterate instances and setIRIS_INST_ERRORundercore->lockbut notinst->lock) could race with this check.Consider moving the early-return check inside the locked section, or document why external serialization makes this safe.
patch/kernel/archive/sm8550-6.13/0019_media--iris--allocate,-initialize-and-queue-intern.patch (1)
307-323:iris_hfi_gen1_buf_type_from_driverreturns-EINVALfrom au32function.The function signature is
static u32, but the default case returns-EINVAL(a negativeint). The callers (lines 353–354, 386–388) compare theu32result against-EINVAL, which works due to implicit unsigned conversion but is a type-safety code smell. Since this is upstream code, noting for awareness only.patch/kernel/archive/sm8550-6.13/0009_media--iris--implement-reqbuf-ioctl-with-vb2_queue.patch (1)
694-694: Minor:u32 default_header = false;uses a boolean literal for au32.While functionally equivalent to
0, usingfalsefor au32variable is misleading. Consideru32 default_header = 0;.
patch/kernel/archive/sm8550-6.13/0016_media--iris--implement-vb2-streaming-ops.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0017_media--iris--implement-set-properties-to-firmware-.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0020_media--iris--implement-vb2-ops-for-buf_queue-and-f.patch
Outdated
Show resolved
Hide resolved
patch/kernel/archive/sm8550-6.13/0033_leds--Add-driver-for-HEROIC-HTR3212.patch
Outdated
Show resolved
Hide resolved
Squishy123
force-pushed
the
added-config-improvements-and-fixes-to-ayn-odin2
branch
from
5a3c1fe to
5168111
Compare
🚫 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. |
@Squishy123 |
Fixed WITH_GRUB flag;
🚫 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. |
🚫 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. |
🚫 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. |
@tabrisnet i have an open PR here |
config/desktop/trixie/environments/kde-plasma-mobile/config_base/packages
Outdated
Show resolved
Hide resolved
…package list changes; rewrote kernel config and duplicated current and edge config for sm8550
🚫 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. |
|
oops typo - fixed now
…On Sat, Feb 21, 2026 at 12:19 AM tabrisnet ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In config/boards/ayn-odin2.csc
<#9316 (comment)>:
> @@ -1,42 +1,66 @@
-# Ayn Odin2 Configuration
+# Qualcomm SM8550 octa core 8GB/12GB/16GB RAM SoC eMMC USB-C WiFi/BT RK3399
huh RK3399?
—
Reply to this email directly, view it on GitHub
<#9316 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB7QE2YNHECVAXCPOFQVIPT4M7THLAVCNFSM6AAAAACTOPBB46VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQMZVGA3DGMBXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
6 participants
Description
General Improvements to Support Ayn Odin2 Device
Changes:
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Used the following config:
Used the following config:
Checklist:
Please delete options that are not relevant.
Summary by CodeRabbit
New Features
Documentation
Chores