sm8250 current: Add support to load QUP SE Firmware via Linux subsystem #8942
Conversation
WalkthroughThis patch series adds Qualcomm QUP SE firmware loading capability to the GENI driver subsystem. It includes devicetree binding updates, register macro refactoring in the core GENI-SE driver, new firmware loading infrastructure, and integration points in I2C, SPI, and UART drivers to dynamically load protocol-specific firmware at runtime when needed. Changes
Sequence Diagram(s)sequenceDiagram
participant Driver as I2C/SPI/UART Driver
participant Core as GENI-SE Core
participant FW as Firmware Loader
participant HW as Hardware (GENI SE)
Driver->>Core: geni_load_se_firmware(se, protocol)
Core->>FW: Request protocol-specific FW
FW-->>Core: Return FW data
Core->>Core: geni_find_protocol_fw (validate header, magic, version)
Core->>Core: geni_configure_xfer_mode (set FIFO/GSI DMA)
Core->>Core: geni_enable_interrupts (setup IRQs)
Core->>HW: Program configuration registers
Core->>HW: Write firmware to GENI SE RAM
Core->>HW: geni_write_fw_revision (update firmware metadata)
Core-->>Driver: Return success/error
alt Success
Driver->>Driver: Continue normal driver setup
else Failure
Driver->>Driver: Log error, cleanup resources
Driver-->>Driver: Return error to probe/init
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Areas requiring extra attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size/large
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patchpatch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patchpatch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch
🔇 Additional comments (8)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch (1)
30-95: QUP SE common DT schema and references look consistent and safe to backportThe new
qcom,se-common-props.yamlschema and the addedallOfreferences in the I2C, SPI, and serial GENI bindings are coherent: the file path matches the$refURI,qcom,enable-gsi-dmais correctly modeled as aflag, andadditionalProperties: truepreserves compatibility with existing device trees. This is a straightforward, upstream‑style binding addition with no obvious 6.12 integration pitfalls.patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch (1)
23-103: GENI SE macro renames are mechanical and align with later firmware workThe macro cleanup (
QUP_HW_VER_REG→QUPV3_HW_VER_REG,GENI_*→SE_GENI_*,DMA_AHB_SLV_CFG_ON→DMA_AHB_SLV_CLK_CGC_ON) is purely mechanical with identical offsets/bit positions, and all in‑file uses are updated (geni_se_get_qup_hw_version,geni_se_io_init). The removed macros were unused in this file, so they should not affect 6.12 builds. This matches what upstream did and looks safe as a base for the subsequent QUP SE firmware‑loading support.patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch (1)
23-33: SPI GENI firmware-load path is consistent; just ensure series orderingThe new
GENI_SE_INVALID_PROTObranch inspi_geni_init()correctly mirrors the I2C/UART pattern: on proto 255 it attemptsgeni_load_se_firmware(se, GENI_SE_SPI)and bails out via the existingout_pmcleanup on failure, while leaving behaviour for valid SPI and other invalid protos unchanged. This looks right for fixing the “Invalid FW loaded, proto: 255” case on SPI.The only integration concern is series ordering: this file assumes that the patch adding
GENI_SE_INVALID_PROTO,GENI_SE_SPI, andgeni_load_se_firmware()(0045) is applied earlier in the sm8250‑6.12 patch stack so the symbols resolve.patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch (1)
23-40: I2C GENI firmware loading and error paths look correct for probe-time useThe new
GENI_SE_INVALID_PROTOhandling ingeni_i2c_probe()is wired cleanly: on invalid proto it attemptsgeni_load_se_firmware(&gi2c->se, GENI_SE_I2C)and, if that fails, logs viadev_err_probe(), turns off SE resources, disablescore_clk, and returns the original error (including -EPROBE_DEFER). The existing non‑I2C proto path is preserved as anelse if, with the same cleanup and-ENXIOas before. This matches the intended behaviour and should fix the “Invalid FW loaded” case without introducing leaks.As with SPI, just ensure the common GENI SE firmware support patch providing
GENI_SE_INVALID_PROTOandgeni_load_se_firmware()is applied earlier in the sm8250‑6.12 patch sequence.patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch (1)
23-36: UART GENI firmware-load handling is minimal and matches SPI/I2C patternsThe updated
qcom_geni_serial_port_setup()now sensibly triesgeni_load_se_firmware(&port->se, GENI_SE_UART)whengeni_se_read_proto()returnsGENI_SE_INVALID_PROTO, only falling back to the existing “Invalid FW loaded”-ENXIOpath for other non‑UART proto values. Error handling on firmware load failure is straightforward (dev_err+ returnret), which is appropriate here. This should eliminate the repeated “proto: 255” messages while keeping behaviour unchanged in all other cases.As with the other drivers, this assumes the GENI SE firmware API and enum extensions are already present from the earlier patch in the series.
patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch (3)
44-103: Macro and header-format definitions look consistent with upstream GENI/QUP usageThe added
protocol_name[],struct se_fw_hdr, MI_PBT* flags, FW/CLOCK/CGC register offsets, andgeni_{set,clr}bits32()helpers are internally consistent and align with how they are used later in this patch. No issues spotted here, assuming the earlier patch in this series providesFW_REV_PROTOCOL_MSKand related register field definitions.Please confirm that the full patch stack (especially the prior qcom-geni-se macro cleanup patch) is applied unmodified so that all new masks (e.g.
FW_REV_PROTOCOL_MSK) are available and the driver still builds cleanly on 6.12.Also applies to: 146-163
295-351: Firmware load sequence, clocks, and resource handling look correctThe transfer-mode configuration (
geni_configure_xfer_mode()), interrupt enables (geni_enable_interrupts()), FW revision write, and the overallgeni_load_se_fw()sequence (ICC set/enable, resources on, RAM programming, and cleanup via theout_resources_off/out_icc_disablelabels) are coherent and appear balanced on both success and failure paths. No additional functional issues stand out here beyond the protocol comparison bug already noted.Given this is a backport, please keep this function in sync with the upstream 6.18 version and confirm that, after fixing the
serial_protocolcomparison, it still behaves identically (or intentionally differs only where 6.12 requires it).Also applies to: 361-494
508-557: Public firmware-loading API and enum extension are sound
geni_load_se_firmware()cleanly wraps property lookup, transfer-mode selection, firmware request, and delegation intogeni_load_se_fw(), returning-EPROBE_DEFERwhen firmware is not yet present. The bounds check againstARRAY_SIZE(protocol_name)ensures safe indexing and preventsGENI_SE_INVALID_PROTO(255) from being used here, while the new enum value provides a clear sentinel for callers.The new declaration in
include/linux/soc/qcom/geni-se.hand the exported symbol match the implementation and intended use in the follow-on I2C/SPI/UART patches.Please double-check that all new call sites in patches 0046–0048 only ever pass concrete protocol values (SPI/UART/I2C/I3C/SPI_SLAVE) and never
GENI_SE_INVALID_PROTO, to keep the bounds and logging assumptions valid.Also applies to: 574-587
| +static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | ||
| + enum geni_se_protocol_type protocol) | ||
| +{ | ||
| + const struct elf32_hdr *ehdr; | ||
| + const struct elf32_phdr *phdrs; | ||
| + const struct elf32_phdr *phdr; | ||
| + struct se_fw_hdr *sefw; | ||
| + u32 fw_end, cfg_idx_end, cfg_val_end; | ||
| + u16 fw_size; | ||
| + int i; | ||
| + | ||
| + if (!fw || fw->size < sizeof(struct elf32_hdr)) | ||
| + return NULL; | ||
| + | ||
| + ehdr = (const struct elf32_hdr *)fw->data; | ||
| + phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | ||
| + | ||
| + /* | ||
| + * The firmware is expected to have at least two program headers (segments). | ||
| + * One for metadata and the other for the actual protocol-specific firmware. | ||
| + */ | ||
| + if (ehdr->e_phnum < 2) { | ||
| + dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + for (i = 0; i < ehdr->e_phnum; i++) { | ||
| + phdr = &phdrs[i]; | ||
| + | ||
| + if (fw->size < phdr->p_offset + phdr->p_filesz) { | ||
| + dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | ||
| + fw->size, phdr->p_offset, phdr->p_filesz); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | ||
| + continue; | ||
| + | ||
| + if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | ||
| + MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | ||
| + continue; | ||
| + | ||
| + if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | ||
| + continue; | ||
| + | ||
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | ||
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | ||
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | ||
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | ||
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | ||
| + | ||
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | ||
| + continue; | ||
| + | ||
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | ||
| + continue; | ||
| + | ||
| + if (fw_size % 2 != 0) { | ||
| + fw_size++; | ||
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | ||
| + } | ||
| + | ||
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | ||
| + dev_err(dev, | ||
| + "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | ||
| + fw_size, MAX_GENI_CFG_RAMn_CNT); | ||
| + continue; | ||
| + } | ||
| + | ||
| + if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | ||
| + cfg_val_end > phdr->p_filesz) { | ||
| + dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | ||
| + continue; | ||
| + } | ||
| + | ||
| + return sefw; | ||
| + } | ||
| + | ||
| + dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | ||
| + return NULL; | ||
| +} |
There was a problem hiding this comment.
Wrong endianness helper on serial_protocol breaks firmware selection
In geni_find_protocol_fw(), serial_protocol is declared as __le16 in struct se_fw_hdr, but the comparison uses le32_to_cpu(sefw->serial_protocol). This:
- Interprets a 16-bit field as 32-bit, reading into the next field (
fw_version). - Makes it effectively impossible for the value to equal the requested
enum geni_se_protocol_type, so the function will never find a matching firmware header and will always fall through to the “Failed to get … protocol firmware” path.
This is a functional blocker for firmware loading.
Apply this minimal fix (matches the field width and other call sites):
- if (le32_to_cpu(sefw->serial_protocol) != protocol)
+ if (le16_to_cpu(sefw->serial_protocol) != protocol)
continue;Please rebuild and re-test QUP SE firmware loading after this change.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | |
| + enum geni_se_protocol_type protocol) | |
| +{ | |
| + const struct elf32_hdr *ehdr; | |
| + const struct elf32_phdr *phdrs; | |
| + const struct elf32_phdr *phdr; | |
| + struct se_fw_hdr *sefw; | |
| + u32 fw_end, cfg_idx_end, cfg_val_end; | |
| + u16 fw_size; | |
| + int i; | |
| + | |
| + if (!fw || fw->size < sizeof(struct elf32_hdr)) | |
| + return NULL; | |
| + | |
| + ehdr = (const struct elf32_hdr *)fw->data; | |
| + phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | |
| + | |
| + /* | |
| + * The firmware is expected to have at least two program headers (segments). | |
| + * One for metadata and the other for the actual protocol-specific firmware. | |
| + */ | |
| + if (ehdr->e_phnum < 2) { | |
| + dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | |
| + return NULL; | |
| + } | |
| + | |
| + for (i = 0; i < ehdr->e_phnum; i++) { | |
| + phdr = &phdrs[i]; | |
| + | |
| + if (fw->size < phdr->p_offset + phdr->p_filesz) { | |
| + dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | |
| + fw->size, phdr->p_offset, phdr->p_filesz); | |
| + return NULL; | |
| + } | |
| + | |
| + if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | |
| + continue; | |
| + | |
| + if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | |
| + MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | |
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | |
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | |
| + continue; | |
| + | |
| + if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | |
| + continue; | |
| + | |
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | |
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | |
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | |
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | |
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | |
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | |
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | |
| + | |
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | |
| + continue; | |
| + | |
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | |
| + continue; | |
| + | |
| + if (fw_size % 2 != 0) { | |
| + fw_size++; | |
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | |
| + } | |
| + | |
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | |
| + dev_err(dev, | |
| + "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | |
| + fw_size, MAX_GENI_CFG_RAMn_CNT); | |
| + continue; | |
| + } | |
| + | |
| + if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | |
| + cfg_val_end > phdr->p_filesz) { | |
| + dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | |
| + continue; | |
| + } | |
| + | |
| + return sefw; | |
| + } | |
| + | |
| + dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | |
| + return NULL; | |
| +} | |
| static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | |
| enum geni_se_protocol_type protocol) | |
| { | |
| const struct elf32_hdr *ehdr; | |
| const struct elf32_phdr *phdrs; | |
| const struct elf32_phdr *phdr; | |
| struct se_fw_hdr *sefw; | |
| u32 fw_end, cfg_idx_end, cfg_val_end; | |
| u16 fw_size; | |
| int i; | |
| if (!fw || fw->size < sizeof(struct elf32_hdr)) | |
| return NULL; | |
| ehdr = (const struct elf32_hdr *)fw->data; | |
| phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | |
| /* | |
| * The firmware is expected to have at least two program headers (segments). | |
| * One for metadata and the other for the actual protocol-specific firmware. | |
| */ | |
| if (ehdr->e_phnum < 2) { | |
| dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | |
| return NULL; | |
| } | |
| for (i = 0; i < ehdr->e_phnum; i++) { | |
| phdr = &phdrs[i]; | |
| if (fw->size < phdr->p_offset + phdr->p_filesz) { | |
| dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | |
| fw->size, phdr->p_offset, phdr->p_filesz); | |
| return NULL; | |
| } | |
| if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | |
| continue; | |
| if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | |
| MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | |
| MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | |
| MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | |
| continue; | |
| if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | |
| continue; | |
| sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | |
| fw_size = le16_to_cpu(sefw->fw_size_in_items); | |
| fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | |
| cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | |
| le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | |
| cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | |
| le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | |
| if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | |
| continue; | |
| if (le16_to_cpu(sefw->serial_protocol) != protocol) | |
| continue; | |
| if (fw_size % 2 != 0) { | |
| fw_size++; | |
| sefw->fw_size_in_items = cpu_to_le16(fw_size); | |
| } | |
| if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | |
| dev_err(dev, | |
| "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | |
| fw_size, MAX_GENI_CFG_RAMn_CNT); | |
| continue; | |
| } | |
| if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | |
| cfg_val_end > phdr->p_filesz) { | |
| dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | |
| continue; | |
| } | |
| return sefw; | |
| } | |
| dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | |
| return NULL; | |
| } |
🤖 Prompt for AI Agents
In
patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch
around lines 199-283, the code incorrectly uses
le32_to_cpu(sefw->serial_protocol) to compare a field declared as __le16,
causing reads to overflow into the next field and preventing any match; change
the comparison to use le16_to_cpu(sefw->serial_protocol) (matching other call
sites and the field width) so the 16-bit serial_protocol is interpreted
correctly and firmware selection can succeed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch (1)
23-40: I2C GENI firmware-load branch is correct; ensure firmware is packaged appropriatelyThe new
proto == GENI_SE_INVALID_PROTObranch ingeni_i2c_probe()correctly triesgeni_load_se_firmware(&gi2c->se, GENI_SE_I2C)and, on failure, usesdev_err_probe()and tears down the SE resources and core clock before returning, mirroring the existing invalid-proto error path preserved in theelse if (proto != GENI_SE_I2C)block. This is logically sound and consistent with the SPI/UART changes.From an Armbian integration standpoint, make sure the corresponding QUP SE firmware blobs are actually installed under
/lib/firmware/updates/and included in the initramfs for sm8250/QCS8250 targets; otherwise these new paths will still fail at probe despite the code changes. Using/lib/firmware/updates/avoids conflicts with the firmware package while remaining first in the kernel firmware search order. Based on learnings.patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch (2)
44-45: Keep protocol_name[] in lockstep with enum geni_se_protocol_typeThe protocol_name[] table assumes that all valid protocol enum values are in the 0..GENI_SE_SPI_SLAVE range, with GENI_SE_INVALID_PROTO as an out-of-range sentinel. That’s a reasonable design, and geni_load_se_firmware() explicitly guards against protocol >= ARRAY_SIZE(protocol_name), so today it’s safe.
To make this future-proof, consider adding a small compile-time check tying the array size to the last “real” protocol enumerator so new protocols can’t be added without updating this table, e.g.:
static const char * const protocol_name[] = { "None", "SPI", "UART", "I2C", "I3C", "SPI SLAVE" }; + +/* Ensure protocol_name[] covers all valid protocol enums (excluding INVALID). */ +static_assert(ARRAY_SIZE(protocol_name) == GENI_SE_SPI_SLAVE + 1);This keeps the mapping robust if more GENI_SE_* values are introduced later, without changing current behavior.
Based on learnings
Also applies to: 573-578
508-555: Firmware load entry point is well-structured; consider minor property handling tweaksgeni_load_se_firmware() is a clean public entry:
- Validates
protocolagainstprotocol_name[]before indexing it.- Retrieves
firmware-namefrom the wrapper device, where it’s naturally specified in DT.- Selects GPI DMA vs FIFO based on
qcom,enable-gsi-dma, but correctly forces FIFO for UART where GSI is unsupported.- Maps
request_firmware()’s-ENOENTto-EPROBE_DEFER, which is appropriate for Armbian’s “firmware must be in initramfs” expectation.- Logs clear error paths and emits a useful dev_dbg() on success.
Two optional refinements you might consider (not blockers):
- Use
device_property_read_bool()instead ofof_property_read_bool()for"qcom,enable-gsi-dma"to also support possible ACPI-based platforms.- Add a newline to the
"Invalid geni-se protocol: %d"dev_err format string for consistent log formatting.These can be deferred; the current behavior is correct for the DT-based QUP use case you’re targeting.
Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: pyavitz
Repo: armbian/build PR: 8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Learnt from: pyavitz
Repo: armbian/build PR: 8481
File: config/kernel/linux-spacemit-edge.config:492-494
Timestamp: 2025-08-11T12:39:22.861Z
Learning: The SpacemiT platform requires esos.elf firmware to be embedded in the kernel image for boot. The Armbian build system handles this by copying esos.elf from $SRC/packages/blobs/riscv64/spacemit/ to ${kernel_work_dir}/firmware/ via the custom_kernel_config__spacemit_k1_firmware() function in config/sources/families/spacemit.conf, ensuring CONFIG_EXTRA_FIRMWARE="esos.elf" can successfully embed it during kernel build.
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patchpatch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch
🔇 Additional comments (7)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch (2)
36-38: Hooking common QUP schema into I2C/SPI/serial bindings looks correctAdding
/schemas/soc/qcom/qcom,se-common-props.yaml#to theallOfblocks forqcom,i2c-geni-qcom.yaml,qcom,serial-geni-qcom.yaml, andqcom,spi-geni-qcom.yamlfollows the usual pattern for sharing peripheral-specific properties and only extends validation to include the new common props (e.g.qcom,enable-gsi-dma) without disturbing the existing controller semantics. This is a clean, upstream-style backport.Also applies to: 48-50, 90-95
60-85: New qcom,se-common-props.yaml schema is minimal, correct, and non-intrusiveThe new schema has matching
$id/file path, standard$schema, a clear title/description, and maintainers.qcom,enable-gsi-dmais correctly modeled as aflagwith a precise description, andadditionalProperties: trueensures the fragment does not over-constrain consumers when composed viaallOf. This should integrate cleanly with existing DT validation.patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch (1)
23-33: SPI GENI firmware-load path on GENI_SE_INVALID_PROTO is consistent and safeThe added
proto == GENI_SE_INVALID_PROTObranch correctly attemptsgeni_load_se_firmware(se, GENI_SE_SPI)and, on failure, logs the error and bails out via the existingout_pmcleanup path. For other unexpected protocol values, the priorproto != GENI_SE_SPIerror handling remains unchanged. This preserves existing behaviour for valid SPI proto while enabling Linux-side firmware loading when the SE starts in the invalid state.patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch (1)
22-103: GENI SE register macro cleanup preserves semantics and aligns naming with later patchesThe macro changes keep all register offsets identical while standardizing names (
QUP_HW_VER_REG→QUPV3_HW_VER_REG,GENI_*→SE_GENI_*) and removing apparently unused GP length / RX length defines.geni_se_get_qup_hw_version()andgeni_se_io_init()are updated to use the new symbols, and theDMA_AHB_SLV_CFG_ON→DMA_AHB_SLV_CLK_CGC_ONrename is correctly reflected in the bitmask write. This is a pure refactor with no functional change and matches the style expected for the subsequent firmware-loading support; keeping it as-is is preferable to avoid diverging from upstream. Based on learnings.patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch (1)
25-35: UART GENI now correctly attempts firmware load when SE protocol is invalidThe new
proto == GENI_SE_INVALID_PROTObranch inqcom_geni_serial_port_setup()cleanly reusesgeni_load_se_firmware(&port->se, GENI_SE_UART)and returns the underlying error on failure, while preserving the existing"Invalid FW loaded, proto: %d"/-ENXIOhandling for other unexpected protocol values. This should resolve the previousproto: 255log spam when firmware is missing while keeping the failure mode unchanged if a wrong (non-UART) firmware is present.patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch (2)
383-494: Resource / ICC sequencing in geni_load_se_fw() looks correctThe overall resource choreography in geni_load_se_fw() looks sound:
geni_icc_set_bw()→geni_icc_enable()→geni_se_resources_on()on the way in.- On all post-resources_on error paths you
goto out_resources_off/out_icc_disable, guaranteeinggeni_se_resources_off()is called only after a successfulgeni_se_resources_on(), and thatgeni_icc_disable()is paired with a successfulgeni_icc_enable().- The success path naturally falls through to the same cleanup labels, so ICC and SE resources are not left enabled after firmware programming finishes.
- Firmware header bounds checks are reused via geni_find_protocol_fw(), and the CFG RAM and primitive-table programming are guarded by the header’s size fields.
From a correctness and leak perspective, this function is in good shape once geni_find_protocol_fw() is hardened as suggested above.
566-587: Header and API are correctly implemented across the patch seriesVerification confirms all three call sites (0046, 0047, 0048 patches) correctly pass valid protocols to
geni_load_se_firmware():
- 0046-i2c: passes
GENI_SE_I2C- 0047-spi: passes
GENI_SE_SPI- 0048-serial: passes
GENI_SE_UART
GENI_SE_INVALID_PROTOis used exclusively as a hardware state sentinel in conditional checks (if (proto == GENI_SE_INVALID_PROTO)), never passed to the function. The implementation meets all stated expectations.
| +/** | ||
| + * geni_find_protocol_fw() - Locate and validate SE firmware for a protocol. | ||
| + * @dev: Pointer to the device structure. | ||
| + * @fw: Pointer to the firmware image. | ||
| + * @protocol: Expected serial engine protocol type. | ||
| + * | ||
| + * Identifies the appropriate firmware image or configuration required for a | ||
| + * specific communication protocol instance running on a Qualcomm GENI | ||
| + * controller. | ||
| + * | ||
| + * Return: pointer to a valid 'struct se_fw_hdr' if found, or NULL otherwise. | ||
| + */ | ||
| +static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | ||
| + enum geni_se_protocol_type protocol) | ||
| +{ | ||
| + const struct elf32_hdr *ehdr; | ||
| + const struct elf32_phdr *phdrs; | ||
| + const struct elf32_phdr *phdr; | ||
| + struct se_fw_hdr *sefw; | ||
| + u32 fw_end, cfg_idx_end, cfg_val_end; | ||
| + u16 fw_size; | ||
| + int i; | ||
| + | ||
| + if (!fw || fw->size < sizeof(struct elf32_hdr)) | ||
| + return NULL; | ||
| + | ||
| + ehdr = (const struct elf32_hdr *)fw->data; | ||
| + phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | ||
| + | ||
| + /* | ||
| + * The firmware is expected to have at least two program headers (segments). | ||
| + * One for metadata and the other for the actual protocol-specific firmware. | ||
| + */ | ||
| + if (ehdr->e_phnum < 2) { | ||
| + dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + for (i = 0; i < ehdr->e_phnum; i++) { | ||
| + phdr = &phdrs[i]; | ||
| + | ||
| + if (fw->size < phdr->p_offset + phdr->p_filesz) { | ||
| + dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | ||
| + fw->size, phdr->p_offset, phdr->p_filesz); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | ||
| + continue; | ||
| + | ||
| + if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | ||
| + MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | ||
| + continue; | ||
| + | ||
| + if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | ||
| + continue; | ||
| + | ||
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | ||
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | ||
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | ||
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | ||
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | ||
| + | ||
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | ||
| + continue; | ||
| + | ||
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | ||
| + continue; | ||
| + | ||
| + if (fw_size % 2 != 0) { | ||
| + fw_size++; | ||
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | ||
| + } | ||
| + | ||
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | ||
| + dev_err(dev, | ||
| + "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | ||
| + fw_size, MAX_GENI_CFG_RAMn_CNT); | ||
| + continue; | ||
| + } | ||
| + | ||
| + if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | ||
| + cfg_val_end > phdr->p_filesz) { | ||
| + dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | ||
| + continue; | ||
| + } | ||
| + | ||
| + return sefw; | ||
| + } | ||
| + | ||
| + dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | ||
| + return NULL; | ||
| +} |
There was a problem hiding this comment.
Harden ELF program-header parsing and fix serial_protocol endianness helper
Two issues in geni_find_protocol_fw() are worth addressing:
- Potential out-of-bounds read on the program header table
You compute phdrs directly from ehdr->e_phoff and then index up to ehdr->e_phnum without checking that the header table actually fits into fw->data:
ehdr = (const struct elf32_hdr *)fw->data;
phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff);
for (i = 0; i < ehdr->e_phnum; i++) {
phdr = &phdrs[i];
if (fw->size < phdr->p_offset + phdr->p_filesz) ...If e_phoff or e_phnum are malicious or corrupt, phdrs[i] can point beyond the firmware buffer, and dereferencing phdr->p_offset / phdr->p_filesz becomes an OOB read in kernel space.
You can close this hole cheaply by validating the program header table before the loop, e.g.:
- ehdr = (const struct elf32_hdr *)fw->data;
- phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff);
+ ehdr = (const struct elf32_hdr *)fw->data;
+
+ if (fw->size < ehdr->e_phoff +
+ (size_t)ehdr->e_phnum * sizeof(*phdrs)) {
+ dev_err(dev, "Invalid firmware: truncated program header table\n");
+ return NULL;
+ }
+
+ phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff);(Exact types/casts can be tuned to match kernel style, but the core idea is to bound-check the table as a whole before touching any phdr fields.)
- Endianness helper for serial_protocol is mismatched to the field type
serial_protocol is declared as __le16 in struct se_fw_hdr, but the comparison uses le32_to_cpu():
struct se_fw_hdr {
__le32 magic;
__le32 version;
__le32 core_version;
__le16 serial_protocol;
...
};
...
if (le32_to_cpu(sefw->serial_protocol) != protocol)
continue;This is type-inconsistent and will either trip sparse/type checking or at least be confusing. Using le16_to_cpu() matches the field width and keeps the code self-documenting:
- if (le32_to_cpu(sefw->serial_protocol) != protocol)
+ if (le16_to_cpu(sefw->serial_protocol) != protocol)
continue;Given this runs in the kernel on untrusted firmware blobs, I’d treat the program-header bounds check as a must-have before merging, even if the code otherwise mirrors upstream, and keep the endianness fix as a straightforward, low-risk correction.
Based on learnings
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch (1)
55-85: Newqcom,se-common-props.yamlschema is well-formedThe new schema has the expected structure (SPDX header,
$id,$schema,title,description,maintainers,properties) and definesqcom,enable-gsi-dmaas a boolean flag via/schemas/types.yaml#/definitions/flag, with a clear description of GSI vs FIFO mode. This should validate cleanly with dt-schema.One minor point to double-check:
additionalProperties: trueat the common-schema level is permissive; this is fine as long as the top-level protocol bindings still enforce their ownadditionalPropertiespolicy. If this matches upstream 6.18, I’d keep it as-is for backport consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patchpatch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patchpatch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch
🔇 Additional comments (13)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch (4)
1-29: Patch metadata is consistent and upstream-alignedCommit header, authorship, Link tag, and summary all look coherent and match an upstream-style dt-bindings patch. Nothing to change here from an Armbian backport perspective.
30-41: I2C GENI binding correctly references the new common schemaAdding
- $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#underallOfcleanly shares the new QUP SE properties with the I2C binding and matches standard dt-bindings patterns. No issues spotted.
43-51: Serial GENI binding integration looks correctExtending the
allOflist with the common QUP SE schema is consistent with the I2C change and keeps SERIAL bindings in sync forqcom,enable-gsi-dma. The placement and path are correct.
86-95: SPI GENI binding change is consistent with I2C/SERIALIncluding the new common props schema in the SPI binding’s
allOflist keeps all three QUP peripherals aligned on shared properties. Path and placement look correct; no further changes needed.patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch (6)
23-28: Copyright block update is consistent and non-invasiveSPDX remains unchanged and the switch to a C-style multi-line comment plus adding the Qualcomm Technologies copyright line is fine; no functional impact. This matches the typical upstream style for these drivers.
Based on learnings
71-76: DMA_AHB_SLV bit rename preserves semanticsRenaming
DMA_AHB_SLV_CFG_ONtoDMA_AHB_SLV_CLK_CGC_ONkeeps the bit position (BIT(2)) unchanged and improves naming consistency with the other*_CLK_CGC_ONbits. No functional change.Based on learnings
79-85: QUP HW version read still targets the same register
geni_se_get_qup_hw_version()now usesQUPV3_HW_VER_REG, which is still0x4, so the read fromwrapper->base + 0x4is identical to the previousQUP_HW_VER_REGusage. No behavior change.Based on learnings
88-103: IO init uses renamed CGC/CLK macros; behavior unchanged
geni_se_io_init()now usesSE_GENI_CGC_CTRLin place ofGENI_CGC_CTRLand the updatedDMA_AHB_SLV_CLK_CGC_ONname, but all register offsets and bit positions are preserved. The same clocks get enabled as before, which is important for the later firmware-loading patches; no regressions expected here.Based on learnings
104-105: Patch footer looks standardThe git version footer (
2.51.1) is normal for an exported upstream patch and doesn’t affect integration into Armbian’s patch system.Based on learnings
36-52: Upstream patch verification confirmed safe; no repository-side verification neededThis patch originates from upstream Linux kernel (commit link:
https://lore.kernel.org/r/[email protected]) and is already merged in the official kernel tree. The armbian/build repository does not contain kernel source code—it applies patches to source fetched during build. Repository-wide verification found no references to the removed macros (GENI_INIT_CFG_REVISION, GENI_S_INIT_CFG_REVISION, GENI_OUTPUT_CTRL, SE_GENI_M_GP_LENGTH, SE_GENI_S_GP_LENGTH, SE_DMA_RX_LEN_IN), confirming upstream maintainers have already ensured their safe removal. The macro renames and removals are safe to apply.patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch (1)
24-39: I2C INVALID_PROTO handling and cleanup look correctThe new
GENI_SE_INVALID_PROTObranch cleanly attempts firmware load, and on failure it logs, tears down SE resources, and disables the core clock before returning the error. The follow‑upelse if (proto != GENI_SE_I2C)preserves the old invalid‑proto behavior, so overall control flow and cleanup remain sound and consistent with the SPI/UART changes in this series.patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch (1)
23-35: SPI INVALID_PROTO handling is consistent with the new SE firmware APIThe added
GENI_SE_INVALID_PROTObranch cleanly integrates SPI with the newgeni_load_se_firmware()path: on success it falls through to the normal flow; on failure it logs once and jumps to the existingout_pmcleanup path. The existing SPI‑slave and invalid‑proto checks (proto != GENI_SE_SPI) remain intact, so overall behavior and resource handling stay consistent.patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch (1)
23-36: UART INVALID_PROTO handling integrates cleanly with SE firmware loadingThe new
GENI_SE_INVALID_PROTOpath inqcom_geni_serial_port_setup()appropriately attempts a UART firmware load and, on failure, logs and returns the error to the caller. The existingproto != GENI_SE_UARTcase is preserved for genuinely wrong protocols. This is consistent with the SPI and I2C patches and should eliminate the previous"Invalid FW loaded, proto: 255"noise once firmware is available.
| #include <linux/acpi.h> | ||
| +#include <linux/bitfield.h> | ||
| #include <linux/clk.h> | ||
| +#include <linux/firmware.h> | ||
| #include <linux/slab.h> | ||
| #include <linux/dma-mapping.h> | ||
| #include <linux/io.h> |
There was a problem hiding this comment.
Add <linux/elf.h> include for ELF32 types
geni_find_protocol_fw() uses struct elf32_hdr and struct elf32_phdr (Line 202–Line 204), but this file only adds <linux/bitfield.h> and <linux/firmware.h> around Line 33–Line 37. Without including <linux/elf.h>, this will not compile.
Please add the missing include next to the other headers, e.g.:
#include <linux/acpi.h>
#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/firmware.h>
+#include <linux/elf.h>
#include <linux/slab.h>Based on learnings.
Also applies to: 199-205
🤖 Prompt for AI Agents
In
patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch
around lines 33 to 39 (and also where similar headers appear around lines 199 to
205), the file uses struct elf32_hdr and struct elf32_phdr but does not include
<linux/elf.h>; add the missing include directive for <linux/elf.h> alongside the
other kernel header includes in both locations so the ELF32 types are defined
and the file compiles.
| +struct se_fw_hdr { | ||
| + __le32 magic; | ||
| + __le32 version; | ||
| + __le32 core_version; | ||
| + __le16 serial_protocol; | ||
| + __le16 fw_version; | ||
| + __le16 cfg_version; | ||
| + __le16 fw_size_in_items; | ||
| + __le16 fw_offset; | ||
| + __le16 cfg_size_in_items; | ||
| + __le16 cfg_idx_offset; | ||
| + __le16 cfg_val_offset; | ||
| +}; |
There was a problem hiding this comment.
Use the correct endianness helper for serial_protocol (__le16)
In struct se_fw_hdr (Line 64–Line 76), serial_protocol is declared as __le16, but in geni_find_protocol_fw() it’s compared using le32_to_cpu (Line 257):
if (le32_to_cpu(sefw->serial_protocol) != protocol)
continue;This reads 32 bits starting at serial_protocol (i.e., mixing in fw_version), so the comparison can fail even for valid firmware once fw_version becomes non‑zero.
This should use le16_to_cpu instead, matching the field type:
- if (le32_to_cpu(sefw->serial_protocol) != protocol)
+ if (le16_to_cpu(sefw->serial_protocol) != protocol)
continue;Based on learnings.
Also applies to: 247-259
🤖 Prompt for AI Agents
In
patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch
around lines 64-76 and usages around 247-259, the struct field serial_protocol
is __le16 but the code compares it using le32_to_cpu (e.g. at line ~257), which
reads 32 bits and can corrupt the comparison; change the conversion to
le16_to_cpu for any comparison or reads of se_fw_hdr.serial_protocol (and
related places that treat it as 16-bit) so the correct 16-bit endianness helper
is used and the comparison uses the proper width.
| + continue; | ||
| + | ||
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | ||
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | ||
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | ||
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | ||
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | ||
| + | ||
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | ||
| + continue; | ||
| + | ||
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | ||
| + continue; | ||
| + | ||
| + if (fw_size % 2 != 0) { | ||
| + fw_size++; | ||
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | ||
| + } | ||
| + | ||
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | ||
| + dev_err(dev, |
There was a problem hiding this comment.
Recompute fw_end after rounding fw_size_in_items to avoid overruns
geni_find_protocol_fw() computes fw_end based on the original fw_size_in_items:
fw_size = le16_to_cpu(sefw->fw_size_in_items);
fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32);
...
if (fw_size % 2 != 0) {
fw_size++;
sefw->fw_size_in_items = cpu_to_le16(fw_size);
}
...
if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz ||
cfg_val_end > phdr->p_filesz)
...Later, geni_load_se_fw() uses hdr->fw_size_in_items when copying to the SE RAM (Line 468–Line 470):
memcpy_toio(se->base + SE_GENI_CFG_RAMN, fw_data,
le16_to_cpu(hdr->fw_size_in_items) * sizeof(u32));If the original fw_size_in_items is odd and just fits within the ELF segment, rounding it up by one increases the copy length, but fw_end and its bounds check are not recomputed. This can make memcpy_toio() read beyond the validated portion of the segment.
Suggestion:
- After adjusting
fw_sizeto even, recomputefw_endand then perform thefw_end > phdr->p_fileszcheck using the updated size.
Example patch:
fw_size = le16_to_cpu(sefw->fw_size_in_items);
fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32);
...
if (fw_size % 2 != 0) {
fw_size++;
sefw->fw_size_in_items = cpu_to_le16(fw_size);
+ fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32);
}
...
- if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz ||
+ if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz ||
cfg_val_end > phdr->p_filesz) {This keeps the behavior while ensuring the bounds check matches the eventual copy size.
Based on learnings.
Also applies to: 468-470
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch(1 hunks)patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patchpatch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch
🔇 Additional comments (21)
patch/kernel/archive/sm8250-6.12/0043-dt-bindings-qcom-se-common-Add-QUP-Peripheral-specif.patch (8)
1-22: Upstream patch metadata is complete and well-formed.The patch header includes all necessary git metadata: proper authorship, co-developer attribution, upstream review tag from Krzysztof Kozlowski (devicetree maintainer), maintainer sign-off from Bjorn Andersson, and a link to the LKML discussion. This confirms the patch has been properly reviewed upstream before backporting.
23-28: File change summary matches the described modifications.The statistics correctly reflect the addition of a new common properties schema and updates to three existing binding files.
30-41: I2C binding schema reference added correctly.The reference to the common properties schema is properly placed in the allOf section with correct YAML syntax and path structure.
42-50: Serial binding schema reference added correctly.Consistent with the I2C binding update, the reference is properly formatted and positioned.
54-86: New common properties schema is well-structured and follows kernel conventions.The YAML schema correctly defines the
qcom,enable-gsi-dmaproperty as a boolean flag to configure GPI DMA mode. TheadditionalProperties: trueon line 85 is appropriate for a reusable schema that will be referenced by other binding files. The schema follows standard kernel devicetree binding documentation patterns with proper licensing, schema URIs, and clear descriptions.
87-97: SPI binding schema reference added correctly.The update is consistent with the I2C and serial binding changes, completing the integration of the common properties schema across all three QUP peripheral types.
98-100: Git version signature present.The patch footer is standard git format-patch output.
1-100: Patch is well-formed and should apply cleanly to 6.12.This is a documentation-only change introducing a new QUP common properties schema and adding schema references to three existing device tree binding files. The binding files (qcom,i2c-geni-qcom.yaml, qcom,spi-geni-qcom.yaml, qcom,serial-geni-qcom.yaml) are properly structured to accept the new
$refadditions.The patch is part of a well-ordered series (0040–0048), has upstream Reviewed-by approval, and contains no syntax errors. No changes required.
patch/kernel/archive/sm8250-6.12/0044-soc-qcom-geni-se-Cleanup-register-defines-and-update.patch (6)
22-28: LGTM!The copyright update correctly adds Qualcomm Technologies, Inc. while preserving the original Linux Foundation copyright. The format change to a block comment is consistent with kernel coding standards.
79-86: LGTM!The update correctly applies the register rename from
QUP_HW_VER_REGtoQUPV3_HW_VER_REG. The function's public API is unchanged, maintaining backward compatibility for external callers.
88-103: LGTM!All register and flag references in
geni_se_io_init()are correctly updated to use the new naming convention. The function logic remains unchanged.
36-67: Request manual verification of macro removals in actual kernel build.This repository contains patches only, not kernel source code. The ripgrep searches found no results because there is no kernel source tree to inspect—not because the macros are unused. I cannot verify whether
GENI_OUTPUT_CTRL,SE_GENI_M_GP_LENGTH,SE_GENI_S_GP_LENGTH, andSE_DMA_RX_LEN_INare referenced elsewhere in the actual 6.12 kernel tree.Verify this patch applies cleanly and the kernel builds successfully with this backport to 6.12, as the safety of the macro removals cannot be determined from this patch archive alone.
71-75: DMA flag rename is correctly applied and consistent.The patch properly updates both the macro definition (
DMA_AHB_SLV_CFG_ON→DMA_AHB_SLV_CLK_CGC_ONat line 74–75) and its usage ingeni_se_io_init()(line 99–100). The new name accurately reflects that this is a clock gating control bit.
3-3: No action required — patch date and upstream source verified as legitimate.The patch was posted to LKML on 2025-09-11 as part of the v7 QUP Serial Engine firmware loading series. The message ID (20250911043256.3523057) encodes the correct date, and the timing aligns with the Linux 6.18 development cycle (merge window mid-October, rc releases late October/November 2025). The patch originates from official upstream LKML and is legitimate.
patch/kernel/archive/sm8250-6.12/0047-spi-geni-qcom-Load-spi-qup-Firmware-from-linux-side.patch (1)
23-33: SPI: INVALID proto firmware-load path is consistent and correctly orderedThe new
GENI_SE_INVALID_PROTObranch callsgeni_load_se_firmware()before the existingproto != GENI_SE_SPIerror, so normal SPI and SPI-slave paths are unchanged while INVALID now gets a chance to self-heal. Error propagation throughretandgoto out_pmshould preserve-EPROBE_DEFERand other errors as-is; just confirm that theout_pmepilogue inspi_geni_init()returnsretunchanged on this platform.patch/kernel/archive/sm8250-6.12/0046-i2c-qcom-geni-Load-i2c-qup-Firmware-from-linux-side.patch (1)
23-40: I2C: INVALID proto recovery and error handling look correct; watch resource balanceThe new
GENI_SE_INVALID_PROTOcase correctly mirrors the SPI/UART pattern: it attemptsgeni_load_se_firmware(&gi2c->se, GENI_SE_I2C)and usesdev_err_probe()so-EPROBE_DEFERis handled properly, then drops SE resources andcore_clkon failure similar to the existing invalid-proto path.One subtlety:
geni_load_se_firmware()manages ICC paths and SE resources internally, whilegeni_i2c_probe()also callsgeni_se_resources_off()/clk_disable_unprepare()on its own error path. This should be safe if those helpers are idempotent (as in upstream), but it's worth confirming in the 6.12 context that resource/clock enable/disable counts remain balanced after a failed firmware load.patch/kernel/archive/sm8250-6.12/0048-serial-qcom-geni-Load-UART-qup-Firmware-from-linux-s.patch (1)
22-36: UART: INVALID proto firmware-load integration is aligned with I2C/SPIThe UART setup now attempts
geni_load_se_firmware(&port->se, GENI_SE_UART)whenproto == GENI_SE_INVALID_PROTOand falls back to the existingInvalid FW loadederror for other unexpected protocols. This matches the pattern used in the I2C/SPI drivers and correctly returns the underlyingretfrom the loader.Please verify that callers of
qcom_geni_serial_port_setup()are fine with seeing-EPROBE_DEFERwhen the UART firmware is not yet available (which is likely desired here so the port reprobes once the firmware is in place).patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch (4)
44-118: Core GENI-SE data structures and register/macro additions look coherentThe new
protocol_name[],struct se_fw_hdr, ELF-related constants, and QUPV3/GENI register/bitfield defines are internally consistent and match the usage later in the file (firmware header parsing, primitive table programming, clock/CGC control, IRQ setup). PlacingGENI_SE_INVALID_PROTO = 255at the end ofenum geni_se_protocol_typealso keeps existing numeric values stable while giving a sentinel for the new INVALID path.Given these definitions are directly consumed by the new firmware loader and by the I2C/SPI/UART call sites, I don’t see any structural issues here for the 6.12 backport.
295-351: Transfer-mode and interrupt configuration helpers look consistent with GENI usage
geni_configure_xfer_mode()cleanly centralizes the FIFO/GPI-DMA/DMA register programming, andgeni_enable_interrupts()sets up M/S and DMA IRQs in a way that matches the masks and register names you define earlier (M_COMMON_GENI_M_IRQ_EN,GENI_SE_DMA_*, etc.). This keeps the mode-specific SE/GSI/DMA wiring out of the protocol drivers and in one place.No functional issues stand out here; just ensure that on 6.12 the SE/GSI event bits (
DMA_RX_EVENT_EN,DMA_TX_EVENT_EN,GENI_M_EVENT_EN,GENI_S_EVENT_EN) and the new DMA IRQ bits still match the hardware/DT configuration for SM8250 as in the 6.18 baseline you’re backporting from.
383-494: Resource/clock/ICC lifecycle in geni_load_se_fw() is subtle but appears self-contained
geni_load_se_fw():
- Locates the protocol-specific firmware with
geni_find_protocol_fw().- Enables ICC bandwidth and paths, powers the SE on (
geni_se_resources_on()), programs QUPV3/GENI clocks/CGC, writes the primitive table/CFG RAM, configures transfer mode, enables interrupts, writes FW revision, and finally toggles SCLK/HCLK and DMA/FIFO interfaces.- Always drops resources and ICC on exit via
geni_se_resources_off()andgeni_icc_disable().Two things to keep in mind for the backport:
Callers (I2C/SPI/UART) must not assume the SE or ICC remain enabled after
geni_load_se_firmware()returns; they should treat it as a self-contained programming transaction and bring resources back up as needed for normal operation. From the snippets you’ve patched, this seems to be the intent, but it’s worth double-checking against the 6.12 versions of those drivers.On early failures before
geni_se_resources_on()succeeds, the function returns directly without callinggeni_icc_disable(), but that matches the typical GENI ICC usage pattern where only a successfulgeni_icc_enable()is paired withgeni_icc_disable().Given your dmesg results and successful bring‑up, the lifecycle looks sound; just ensure there are no remaining 6.12-only callers that expected
resources_on()to keep the SE powered beyond this programming step.
508-555: Public API geni_load_se_firmware() and GENI_SE_INVALID_PROTO integration look goodThe exported
geni_load_se_firmware():
- Validates the
protocolagainstprotocol_name[].- Reads a required
firmware-nameproperty from the wrapper device.- Optionally enables GSI DMA via
qcom,enable-gsi-dma, but forces FIFO mode for UART.- Requests the firmware and maps
-ENOENTto-EPROBE_DEFERso protocol drivers can reprobe once the blob is available.- Delegates the actual programming to
geni_load_se_fw()and logs detailed errors.The header update (
GENI_SE_INVALID_PROTO = 255and the prototype) lines up with how the I2C/SPI/UART drivers now trigger firmware loading whengeni_se_read_proto()reports INVALID.Given Armbian’s preference to track upstream patches closely, this public API shape looks appropriate for the 6.12 backport. The main thing to verify is that all SM8250 QUP wrapper nodes used here have the correct
firmware-nameproperty and that the corresponding firmware files are included in the initramfs, otherwise you’ll see persistent-EPROBE_DEFERand probe loops. Based on learnings.Also applies to: 566-587
| +/** | ||
| + * geni_find_protocol_fw() - Locate and validate SE firmware for a protocol. | ||
| + * @dev: Pointer to the device structure. | ||
| + * @fw: Pointer to the firmware image. | ||
| + * @protocol: Expected serial engine protocol type. | ||
| + * | ||
| + * Identifies the appropriate firmware image or configuration required for a | ||
| + * specific communication protocol instance running on a Qualcomm GENI | ||
| + * controller. | ||
| + * | ||
| + * Return: pointer to a valid 'struct se_fw_hdr' if found, or NULL otherwise. | ||
| + */ | ||
| +static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | ||
| + enum geni_se_protocol_type protocol) | ||
| +{ | ||
| + const struct elf32_hdr *ehdr; | ||
| + const struct elf32_phdr *phdrs; | ||
| + const struct elf32_phdr *phdr; | ||
| + struct se_fw_hdr *sefw; | ||
| + u32 fw_end, cfg_idx_end, cfg_val_end; | ||
| + u16 fw_size; | ||
| + int i; | ||
| + | ||
| + if (!fw || fw->size < sizeof(struct elf32_hdr)) | ||
| + return NULL; | ||
| + | ||
| + ehdr = (const struct elf32_hdr *)fw->data; | ||
| + phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | ||
| + | ||
| + /* | ||
| + * The firmware is expected to have at least two program headers (segments). | ||
| + * One for metadata and the other for the actual protocol-specific firmware. | ||
| + */ | ||
| + if (ehdr->e_phnum < 2) { | ||
| + dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + for (i = 0; i < ehdr->e_phnum; i++) { | ||
| + phdr = &phdrs[i]; | ||
| + | ||
| + if (fw->size < phdr->p_offset + phdr->p_filesz) { | ||
| + dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | ||
| + fw->size, phdr->p_offset, phdr->p_filesz); | ||
| + return NULL; | ||
| + } | ||
| + | ||
| + if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | ||
| + continue; | ||
| + | ||
| + if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | ||
| + MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | ||
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | ||
| + continue; | ||
| + | ||
| + if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | ||
| + continue; | ||
| + | ||
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | ||
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | ||
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | ||
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | ||
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | ||
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | ||
| + | ||
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | ||
| + continue; | ||
| + | ||
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | ||
| + continue; | ||
| + | ||
| + if (fw_size % 2 != 0) { | ||
| + fw_size++; | ||
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | ||
| + } | ||
| + | ||
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | ||
| + dev_err(dev, | ||
| + "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | ||
| + fw_size, MAX_GENI_CFG_RAMn_CNT); | ||
| + continue; | ||
| + } | ||
| + | ||
| + if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | ||
| + cfg_val_end > phdr->p_filesz) { | ||
| + dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | ||
| + continue; | ||
| + } | ||
| + | ||
| + return sefw; | ||
| + } | ||
| + | ||
| + dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | ||
| + return NULL; | ||
| +} |
There was a problem hiding this comment.
🧩 Analysis chain
Minor endianness bug and potential hardening opportunity in geni_find_protocol_fw()
geni_find_protocol_fw() does a good job of validating ELF segments and guarding against truncated SE firmware segments, but there are two points worth tightening:
- The protocol compare currently uses
le32_to_cpu(sefw->serial_protocol)even thoughserial_protocolis declared as__le16. On little‑endian ARM this happens to work, but it’s type‑mismatched and would break on big‑endian. Usingle16_to_cpu()here is the correct and clearer choice:
- if (le32_to_cpu(sefw->serial_protocol) != protocol)
+ if (le16_to_cpu(sefw->serial_protocol) != protocol)
continue;- For extra robustness you might also want to add a bounds check that
ehdr->e_phoff + e_phnum * sizeof(*phdrs) <= fw->sizebefore computingphdrs, to avoid ever indexing past the firmware blob if a malformed image slips through. This is probably low‑risk given the firmware is Qualcomm‑supplied, so consider it optional, especially if you prefer to keep this patch identical to the upstream 6.18 version and instead carry hardening changes in a tiny follow‑up patch. Based on learnings.
I'll help you verify the review comment. Let me examine the patch file and the structure definitions to confirm the identified issues.
I need to examine the actual patch file in the repository to verify the specific issues. Let me search for it.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find and examine the patch file
find . -name "*0045-soc-qcom-geni-se*" -type f 2>/dev/null | head -5Length of output: 98
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Extract lines around 187-283 and search for relevant structures
echo "=== Extracting lines 187-283 from patch ==="
sed -n '187,283p' ./patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch
echo -e "\n=== Searching for se_fw_hdr structure ==="
grep -n "se_fw_hdr" ./patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch | head -20Length of output: 3719
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Extract the se_fw_hdr structure definition
echo "=== Extracting se_fw_hdr structure definition ==="
sed -n '47,85p' ./patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patch
echo -e "\n=== Looking for bounds check before phdrs ==="
sed -n '210,230p' ./patch/kernel/archive/sm8250-6.12/0045-soc-qcom-geni-se-Add-support-to-load-QUP-SE-Firmware.patchLength of output: 2086
Fix type mismatch in endianness conversion for serial_protocol field
The serial_protocol field is declared as __le16 in the se_fw_hdr structure (line ~70), but the code at line ~255 uses le32_to_cpu(sefw->serial_protocol). This type mismatch works by accident on little-endian ARM but will break on big-endian systems and reads from incorrect memory offsets. Change to le16_to_cpu():
- if (le32_to_cpu(sefw->serial_protocol) != protocol)
+ if (le16_to_cpu(sefw->serial_protocol) != protocol)
continue;Additionally, consider adding a bounds check before computing phdrs at line ~215 to validate that ehdr->e_phoff + ehdr->e_phnum * sizeof(*phdrs) <= fw->size, though this is optional hardening since the firmware is Qualcomm-supplied.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +/** | |
| + * geni_find_protocol_fw() - Locate and validate SE firmware for a protocol. | |
| + * @dev: Pointer to the device structure. | |
| + * @fw: Pointer to the firmware image. | |
| + * @protocol: Expected serial engine protocol type. | |
| + * | |
| + * Identifies the appropriate firmware image or configuration required for a | |
| + * specific communication protocol instance running on a Qualcomm GENI | |
| + * controller. | |
| + * | |
| + * Return: pointer to a valid 'struct se_fw_hdr' if found, or NULL otherwise. | |
| + */ | |
| +static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | |
| + enum geni_se_protocol_type protocol) | |
| +{ | |
| + const struct elf32_hdr *ehdr; | |
| + const struct elf32_phdr *phdrs; | |
| + const struct elf32_phdr *phdr; | |
| + struct se_fw_hdr *sefw; | |
| + u32 fw_end, cfg_idx_end, cfg_val_end; | |
| + u16 fw_size; | |
| + int i; | |
| + | |
| + if (!fw || fw->size < sizeof(struct elf32_hdr)) | |
| + return NULL; | |
| + | |
| + ehdr = (const struct elf32_hdr *)fw->data; | |
| + phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | |
| + | |
| + /* | |
| + * The firmware is expected to have at least two program headers (segments). | |
| + * One for metadata and the other for the actual protocol-specific firmware. | |
| + */ | |
| + if (ehdr->e_phnum < 2) { | |
| + dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | |
| + return NULL; | |
| + } | |
| + | |
| + for (i = 0; i < ehdr->e_phnum; i++) { | |
| + phdr = &phdrs[i]; | |
| + | |
| + if (fw->size < phdr->p_offset + phdr->p_filesz) { | |
| + dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | |
| + fw->size, phdr->p_offset, phdr->p_filesz); | |
| + return NULL; | |
| + } | |
| + | |
| + if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | |
| + continue; | |
| + | |
| + if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | |
| + MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | |
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | |
| + MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | |
| + continue; | |
| + | |
| + if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | |
| + continue; | |
| + | |
| + sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | |
| + fw_size = le16_to_cpu(sefw->fw_size_in_items); | |
| + fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | |
| + cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | |
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | |
| + cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | |
| + le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | |
| + | |
| + if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | |
| + continue; | |
| + | |
| + if (le32_to_cpu(sefw->serial_protocol) != protocol) | |
| + continue; | |
| + | |
| + if (fw_size % 2 != 0) { | |
| + fw_size++; | |
| + sefw->fw_size_in_items = cpu_to_le16(fw_size); | |
| + } | |
| + | |
| + if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | |
| + dev_err(dev, | |
| + "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | |
| + fw_size, MAX_GENI_CFG_RAMn_CNT); | |
| + continue; | |
| + } | |
| + | |
| + if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | |
| + cfg_val_end > phdr->p_filesz) { | |
| + dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | |
| + continue; | |
| + } | |
| + | |
| + return sefw; | |
| + } | |
| + | |
| + dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | |
| + return NULL; | |
| +} | |
| /** | |
| * geni_find_protocol_fw() - Locate and validate SE firmware for a protocol. | |
| * @dev: Pointer to the device structure. | |
| * @fw: Pointer to the firmware image. | |
| * @protocol: Expected serial engine protocol type. | |
| * | |
| * Identifies the appropriate firmware image or configuration required for a | |
| * specific communication protocol instance running on a Qualcomm GENI | |
| * controller. | |
| * | |
| * Return: pointer to a valid 'struct se_fw_hdr' if found, or NULL otherwise. | |
| */ | |
| static struct se_fw_hdr *geni_find_protocol_fw(struct device *dev, const struct firmware *fw, | |
| enum geni_se_protocol_type protocol) | |
| { | |
| const struct elf32_hdr *ehdr; | |
| const struct elf32_phdr *phdrs; | |
| const struct elf32_phdr *phdr; | |
| struct se_fw_hdr *sefw; | |
| u32 fw_end, cfg_idx_end, cfg_val_end; | |
| u16 fw_size; | |
| int i; | |
| if (!fw || fw->size < sizeof(struct elf32_hdr)) | |
| return NULL; | |
| ehdr = (const struct elf32_hdr *)fw->data; | |
| phdrs = (const struct elf32_phdr *)(fw->data + ehdr->e_phoff); | |
| /* | |
| * The firmware is expected to have at least two program headers (segments). | |
| * One for metadata and the other for the actual protocol-specific firmware. | |
| */ | |
| if (ehdr->e_phnum < 2) { | |
| dev_err(dev, "Invalid firmware: less than 2 program headers\n"); | |
| return NULL; | |
| } | |
| for (i = 0; i < ehdr->e_phnum; i++) { | |
| phdr = &phdrs[i]; | |
| if (fw->size < phdr->p_offset + phdr->p_filesz) { | |
| dev_err(dev, "Firmware size (%zu) < expected offset (%u) + size (%u)\n", | |
| fw->size, phdr->p_offset, phdr->p_filesz); | |
| return NULL; | |
| } | |
| if (phdr->p_type != PT_LOAD || !phdr->p_memsz) | |
| continue; | |
| if (MI_PBT_PAGE_MODE_VALUE(phdr->p_flags) != MI_PBT_NON_PAGED_SEGMENT || | |
| MI_PBT_SEGMENT_TYPE_VALUE(phdr->p_flags) == MI_PBT_HASH_SEGMENT || | |
| MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_NOTUSED_SEGMENT || | |
| MI_PBT_ACCESS_TYPE_VALUE(phdr->p_flags) == MI_PBT_SHARED_SEGMENT) | |
| continue; | |
| if (phdr->p_filesz < sizeof(struct se_fw_hdr)) | |
| continue; | |
| sefw = (struct se_fw_hdr *)(fw->data + phdr->p_offset); | |
| fw_size = le16_to_cpu(sefw->fw_size_in_items); | |
| fw_end = le16_to_cpu(sefw->fw_offset) + fw_size * sizeof(u32); | |
| cfg_idx_end = le16_to_cpu(sefw->cfg_idx_offset) + | |
| le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u8); | |
| cfg_val_end = le16_to_cpu(sefw->cfg_val_offset) + | |
| le16_to_cpu(sefw->cfg_size_in_items) * sizeof(u32); | |
| if (le32_to_cpu(sefw->magic) != SE_MAGIC_NUM || le32_to_cpu(sefw->version) != 1) | |
| continue; | |
| if (le16_to_cpu(sefw->serial_protocol) != protocol) | |
| continue; | |
| if (fw_size % 2 != 0) { | |
| fw_size++; | |
| sefw->fw_size_in_items = cpu_to_le16(fw_size); | |
| } | |
| if (fw_size >= MAX_GENI_CFG_RAMn_CNT) { | |
| dev_err(dev, | |
| "Firmware size (%u) exceeds max allowed RAMn count (%u)\n", | |
| fw_size, MAX_GENI_CFG_RAMn_CNT); | |
| continue; | |
| } | |
| if (fw_end > phdr->p_filesz || cfg_idx_end > phdr->p_filesz || | |
| cfg_val_end > phdr->p_filesz) { | |
| dev_err(dev, "Truncated or corrupt SE FW segment found at index %d\n", i); | |
| continue; | |
| } | |
| return sefw; | |
| } | |
| dev_err(dev, "Failed to get %s protocol firmware\n", protocol_name[protocol]); | |
| return NULL; | |
| } |
|
✅ This PR has been reviewed and approved — all set for merge! |
3 participants
Description
The backported patch from Linux 6.18 allows loading QUP firmware under the Linux system.
LKML url:
https://lore.kernel.org/all/[email protected]/
The commit for each patch in the GitHub Linux repository:
torvalds/linux@9bc7130
torvalds/linux@b44a593
torvalds/linux@d4bf065
torvalds/linux@b645df7
torvalds/linux@99cf351
torvalds/linux@3f17073
By appending .patch to the end of these URLs, you get the patches in this PR. The qup i2c patch had some minor errors under the 6.12 kernel, and I have corrected them.
How Has This Been Tested?
use
./compile.sh BOARD=damo-cockpit8250 BRANCH=current RELEASE=nobleto compile the image for my own QCS8250 device.Before applying the patches:
After applying the patches:
It is worth noting that the qup firmware needs to be added to the initramfs.
For the mainline armbian/build source code, use
./compile.sh BOARD=xiaomi-elish BRANCH=current RELEASE=nobleto ensure the kernel compiles successfully.Why can't I build a complete xiaomi-elish image? Because the official mkbootimg in Noble seems to have a bug.For my own device, there is a temporary fix.
Checklist:
Summary by CodeRabbit
New Features
Documentation
Refactor