sunxi a523: Adding support for PCIe, SPI Flash and Iommu #9280
Conversation
rvdr
requested review from
AGM1968,
JohnTheCoolingFan,
PanderMusubi,
TheSnowfield,
Tonymac32,
chraac,
igorpecovnik,
janprunk,
krachlatte,
pyavitz,
schwar3kat,
sgjava and
teknoid
as code owners
|
This needn't be done yet, but a note: see if you can consolidate commits into logically related things [with meaningful commit messages] prior to merge. |
|
Tip For best results, initiate chat on the files or code changes.
These could potentially be consolidated into commits like:
Happy to assist with organizing when ready! 🐰 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes.
Feel free to ping me when you're ready to organize the commits, and I can help review the proposed structure or provide more specific guidance based on Armbian conventions. Happy to assist further! 🐰 🧠 Learnings used |
|
Yes, would be a nice to have. Otherwise I'd squash everything. |
![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
🤖 Fix all issues with AI agents
In
`@patch/kernel/archive/sunxi-6.18/patches.armbian/drv-phy-allwinner-add-pcie-usb3-driver.patch`:
- Around line 309-339: The clk_set_rate() calls for combphy->phyclk_ref and
combphy->phyclk_cfg must be guarded against ERR_PTR; before calling
clk_set_rate(combphy->phyclk_ref, 100000000) and
clk_set_rate(combphy->phyclk_cfg, 200000000) check !IS_ERR(combphy->phyclk_ref)
and !IS_ERR(combphy->phyclk_cfg) respectively (matching the existing
clk_prepare_enable guards) so you don't call clk_set_rate on an ERR_PTR and risk
a crash; keep the existing error logging and existing goto error paths
unchanged.
♻️ Duplicate comments (16)
patch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch (5)
589-593: CFG0/CFG1 type selection appears inverted.For DesignWare PCIe controllers, Type 0 configuration (CFG0) is used to access devices on the immediate downstream bus, and Type 1 (CFG1) for devices on further buses. The current logic uses CFG0 when
PCI_BUS(d) != 0and CFG1 otherwise, which seems backwards for typical RC enumeration.This was flagged in a previous review. If this is intentionally different for sun55i hardware, please confirm and document the rationale.
682-687: Missing indentation forsize_len = 2;.The assignment on line 685 is missing its leading tab.
🔧 Suggested fix
if (size == PCI_SIZE_8) size_len = 1; else if (size == PCI_SIZE_16) - size_len = 2; + size_len = 2; else if (size == PCI_SIZE_32) size_len = 4;
847-851: LNKCTL2 access should use discovered capability offset.The code finds the PCI Express capability offset at line 840 but then accesses
LINK_CONTROL2_LINK_STATUS2(a hardcoded offset of 0xa0) directly instead of usingoffset + PCI_EXP_LNKCTL2. If the capability isn't at the expected location, this will program the wrong register.🔧 Suggested fix
- val = sun55i_pcie_readl_dbi(pci, LINK_CONTROL2_LINK_STATUS2); + val = sun55i_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2); val &= ~PCI_EXP_LNKCTL2_TLS; val |= gen; - sun55i_pcie_writel_dbi(pci, LINK_CONTROL2_LINK_STATUS2, val); + sun55i_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, val);
1088-1096: Duplicate macro definitions will cause preprocessor warnings.
PCIE_ATU_TYPE_MEM,PCIE_ATU_TYPE_IO,PCIE_ATU_TYPE_CFG0,PCIE_ATU_TYPE_CFG1are defined at lines 1072-1075 and again at lines 1088-1091.PCIE_ATU_FUNC_NUMis defined at both line 1092 and 1096.🔧 Suggested fix
Remove the duplicate definitions at lines 1088-1091 and the duplicate at line 1096:
`#define` PCIE_ATU_CR1_INBOUND(reg) (0x300100 + ((reg) * 0x200)) -#define PCIE_ATU_TYPE_MEM (0x0 << 0) -#define PCIE_ATU_TYPE_IO (0x2 << 0) -#define PCIE_ATU_TYPE_CFG0 (0x4 << 0) -#define PCIE_ATU_TYPE_CFG1 (0x5 << 0) `#define` PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) `#define` PCIE_ATU_CR2_INBOUND(reg) (0x300104 + ((reg) * 0x200)) `#define` PCIE_ATU_MATCH_MODE BIT(30) `#define` PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) -#define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
373-377: PHY power-on failure skips power-off cleanup.If
generic_phy_power_on()fails, the code jumps toerr_assert_reset, skippinggeneric_phy_power_off(). Sincegeneric_phy_init()succeeded at this point, the PHY may be left in a partially initialized state.🔧 Suggested fix
ret = generic_phy_power_on(&pci->phy); if (ret) { printf("PCIe: Failed to power on phy: %d\n", ret); - goto err_assert_reset; + goto err_exit_phy; }And add a new label before
err_power_off_phy:+err_exit_phy: + generic_phy_exit(&pci->phy); err_power_off_phy: generic_phy_power_off(&pci->phy);patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch (7)
1382-1386: Translate/remove non‑English inline comments.
Kernel style expects English; there are still German notes (e.g., “Wie in Fehler 1 korrigiert”) that should be removed or translated.
1414-1438: Guard args_count before using args[1].
The binding shows#iommu-cells = <1>, soargs_countcan be 1; readingargs[1]is an out‑of‑bounds access.🐛 Proposed fix
static int sun55i_iommu_of_xlate(struct device *dev, const struct of_phandle_args *args) { struct sunxi_iommu_owner *owner = dev_iommu_priv_get(dev); struct platform_device *sysmmu = of_find_device_by_node(args->np); struct sunxi_iommu_dev *data; if (!sysmmu) return -ENODEV; data = platform_get_drvdata(sysmmu); if (data == NULL) { put_device(&sysmmu->dev); return -ENODEV; } + if (args->args_count < 1) { + put_device(&sysmmu->dev); + return -EINVAL; + } if (!owner) { owner = kzalloc(sizeof(*owner), GFP_KERNEL); if (!owner) { put_device(&sysmmu->dev); return -ENOMEM; } owner->tlbid = args->args[0]; - owner->flag = args->args[1]; + owner->flag = (args->args_count > 1) ? args->args[1] : 0; owner->data = data; owner->dev = dev; dev_iommu_priv_set(dev, owner); }
1509-1518: Validate master_id before array indexing in IRQ handler.
master_idis derived from a bitmap and used to index fixed arrays without bounds checks, risking OOB access.🐛 Proposed fix
if (!(int_masterid_bitmap & (1U << 31))) { int_masterid_bitmap &= 0xffff; + if (!int_masterid_bitmap) + goto clear_irq; master_id = __ffs(int_masterid_bitmap); + if (master_id >= ARRAY_SIZE(sunxi_iommu_fault_notify_cbs) || + master_id >= ARRAY_SIZE(plat_data->master)) { + pr_err("sunxi iommu: invalid master_id %u\n", master_id); + goto clear_irq; + } pr_err("Bug is in %s module, invalid address: 0x%x, data:0x%x, id:0x%x\n", plat_data->master[master_id], addr_reg, data_reg, int_masterid_bitmap); /* master debug callback */ if (sunxi_iommu_fault_notify_cbs[master_id]) sunxi_iommu_fault_notify_cbs[master_id](); } +clear_irq: /* invalid TLB */
2051-2059: Free iopte_cache on devm_kzalloc failure.
Ifdevm_kzallocfails,iopte_cacheleaks.🐛 Proposed fix
sunxi_iommu = devm_kzalloc(dev, sizeof(*sunxi_iommu), GFP_KERNEL); - if (!sunxi_iommu) - return -ENOMEM; + if (!sunxi_iommu) { + sunxi_pgtable_free_pte_cache(iopte_cache); + return -ENOMEM; + }
2094-2099: Preserve the clock error code before NULLing.
PTR_ERRis called after settingclkto NULL, which zeroes the error code.🐛 Proposed fix
sunxi_iommu->clk = of_clk_get_by_name(dev->of_node, "iommu"); if (IS_ERR(sunxi_iommu->clk)) { - sunxi_iommu->clk = NULL; - dev_dbg(dev, "Unable to find clock\n"); - ret = PTR_ERR(sunxi_iommu->clk); /* Korrekter Fehlercode */ + ret = PTR_ERR(sunxi_iommu->clk); + sunxi_iommu->clk = NULL; + dev_dbg(dev, "Unable to find clock\n"); goto err_clk; }
2118-2124: Remove custom sysfs files if iommu_device_sysfs_add fails.
The custom sysfs attributes are created before the core sysfs registration; on failure they’re left behind.🐛 Proposed fix
ret = iommu_device_sysfs_add(&sunxi_iommu->iommu, dev, NULL, dev_name(dev)); if (ret) { dev_err(dev, "Failed to register iommu in sysfs\n"); + sun55i_iommu_sysfs_remove(pdev); clk_disable_unprepare(sunxi_iommu->clk); goto err_clk; }
2208-2211: Use per‑device context in suspend.
suspend()should usedev_get_drvdata(dev)instead of the global to avoid NULL deref or cross‑device issues.🐛 Proposed fix
static int sun55i_iommu_suspend(struct device *dev) { - clk_disable_unprepare(global_iommu_dev->clk); + struct sunxi_iommu_dev *iommu = dev_get_drvdata(dev); + + if (!iommu || !iommu->clk) + return 0; + clk_disable_unprepare(iommu->clk); return 0; }patch/kernel/archive/sunxi-6.18/patches.armbian/drv-phy-allwinner-add-pcie-usb3-driver.patch (4)
203-228: Fix bit update logic incombo_phy_mode_set.
tmpstarts at 0, so clearing bits is ineffective and the disable path never clearsPHY_RSTN/mode bits. This can leave stale state when switching modes.🐛 Proposed fix
- u32 val, tmp = 0; - - val = readl(COMBO_REG_PHYCTRL(combphy->phy_ctl)); - - if (combphy->user == PHY_USE_BY_PCIE) - tmp &= ~PHY_USE_SEL; - else if (combphy->user == PHY_USE_BY_USB3) - tmp |= PHY_USE_SEL; - else if (combphy->user == PHY_USE_BY_PCIE_USB3_U2) - tmp &= ~PHY_USE_SEL; - - if (combphy->ref == INTER_SIG_REF_CLK) - tmp &= ~PHY_CLK_SEL; - else if (combphy->ref == EXTER_DIF_REF_CLK) - tmp |= PHY_CLK_SEL; - - if (enable) { - tmp |= PHY_RSTN; - val |= tmp; - } else { - tmp &= ~PHY_RSTN; - val &= ~tmp; - } - writel(val, COMBO_REG_PHYCTRL(combphy->phy_ctl)); + u32 val; + + val = readl(COMBO_REG_PHYCTRL(combphy->phy_ctl)); + + val &= ~PHY_USE_SEL; + if (combphy->user == PHY_USE_BY_USB3) + val |= PHY_USE_SEL; + + val &= ~PHY_CLK_SEL; + if (combphy->ref == EXTER_DIF_REF_CLK) + val |= PHY_CLK_SEL; + + if (enable) + val |= PHY_RSTN; + else + val &= ~PHY_RSTN; + + writel(val, COMBO_REG_PHYCTRL(combphy->phy_ctl));
953-979: Avoidclk_set_parent()with ERR_PTR clocks.
phyclk_ref/refclk_parandphyclk_cfg/cfgclk_parare only dev_dbg’d on failure, yet always passed toclk_set_parent(). That can crash if a clock is missing.🐛 Proposed fix
combphy->refclk_par = devm_clk_get(&pdev->dev, "refclk_par"); if (IS_ERR(combphy->refclk_par)) - dev_dbg(dev, "failed to get refclk_par\n"); + return PTR_ERR(combphy->refclk_par); + if (IS_ERR(combphy->phyclk_ref)) + return PTR_ERR(combphy->phyclk_ref); ret = clk_set_parent(combphy->phyclk_ref, combphy->refclk_par); if (ret) { dev_err(dev, "failed to set refclk parent\n"); return -EINVAL; } @@ combphy->phyclk_cfg = devm_clk_get(&pdev->dev, "phyclk_cfg"); if (IS_ERR(combphy->phyclk_cfg)) - dev_dbg(dev, "failed to get phyclk_cfg\n"); + return PTR_ERR(combphy->phyclk_cfg); combphy->cfgclk_par = devm_clk_get(&pdev->dev, "cfgclk_par"); if (IS_ERR(combphy->cfgclk_par)) - dev_dbg(dev, "failed to get cfgclk_par\n"); + return PTR_ERR(combphy->cfgclk_par); ret = clk_set_parent(combphy->phyclk_cfg, combphy->cfgclk_par);
863-872: Propagate USB3 power-on failures.The return from
sunxi_combphy_usb3_power_on()is ignored, so regulator failures are masked.🐛 Proposed fix
static int sunxi_combphy_power_on(struct phy *phy) { struct sunxi_combphy *combphy = phy_get_drvdata(phy); + int ret; switch (combphy->mode) { case PHY_TYPE_PCIE: break; case PHY_TYPE_USB3: - sunxi_combphy_usb3_power_on(combphy); + ret = sunxi_combphy_usb3_power_on(combphy); + if (ret) + return ret; break; default: dev_err(combphy->dev, "incompatible PHY type\n"); return -EINVAL;
944-951: Unconditional dereference of error pointers from failed reset control retrieval.If
devm_reset_control_get()fails at lines 944–950, an ERR_PTR is stored but not checked. This error pointer is then passed unconditionally toreset_control_deassert(combphy->reset)at line 298 andreset_control_deassert(combphy->noppu_reset)at line 303, causing undefined behavior. The error path at lines 374–377 also callsreset_control_assert()without IS_ERR checks, compounding the issue.Return the error instead of logging to prevent initialization from proceeding with invalid reset control pointers.
Proposed fix
combphy->reset = devm_reset_control_get(dev, "phy_rst"); if (IS_ERR(combphy->reset)) - dev_dbg(dev, "failed to get reset control\n"); + return PTR_ERR(combphy->reset); if (combphy->drvdata->need_noppu_rst) { combphy->noppu_reset = devm_reset_control_get(dev, "noppu_rst"); if (IS_ERR(combphy->noppu_reset)) - dev_dbg(dev, "failed to get noppu_reset control\n"); + return PTR_ERR(combphy->noppu_reset); }
🧹 Nitpick comments (5)
patch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch (4)
96-118: Inconsistent indentation style.This function uses 4-space indentation while the rest of the file uses tabs. U-Boot coding style requires tabs for indentation.
🔧 Suggested fix
int sun55i_pcie_cfg_read(void __iomem *addr, int size, ulong *val) { - - if ((uintptr_t)addr & (size - 1)) { - *val = 0; - printf("CFG_READ: Bad alignment\n"); - return PCIBIOS_BAD_REGISTER_NUMBER; - } - - if (size == 4) { - *val = readl(addr); - } else if (size == 2) { - *val = readw(addr); - } else if (size == 1) { - *val = readb(addr); - } else { - *val = 0; - printf("CFG_READ: Bad size\n"); - return PCIBIOS_BAD_REGISTER_NUMBER; - } - - return PCIBIOS_SUCCESSFUL; + if ((uintptr_t)addr & (size - 1)) { + *val = 0; + printf("CFG_READ: Bad alignment\n"); + return PCIBIOS_BAD_REGISTER_NUMBER; + } + + if (size == 4) + *val = readl(addr); + else if (size == 2) + *val = readw(addr); + else if (size == 1) + *val = readb(addr); + else { + *val = 0; + printf("CFG_READ: Bad size\n"); + return PCIBIOS_BAD_REGISTER_NUMBER; + } + + return PCIBIOS_SUCCESSFUL; }
773-778: Unconditional debug output in link status check.These
printfstatements will output on every link status check, which can be noisy during normal operation. Consider gating them behind a debug flag or removing them.🔧 Suggested fix
- printf(" Link Status: 0x%08x\n", val); - printf(" RDLH_LINK_UP: %d\n", !!(val & RDLH_LINK_UP)); - printf(" SMLH_LINK_UP: %d\n", !!(val & SMLH_LINK_UP)); - printf(" LINK_SPEED: %d\n", (val >> 16) & 0xF); - printf(" LINK_WIDTH: %d\n", (val >> 20) & 0x3F); + debug(" Link Status: 0x%08x\n", val); + debug(" RDLH_LINK_UP: %d\n", !!(val & RDLH_LINK_UP)); + debug(" SMLH_LINK_UP: %d\n", !!(val & SMLH_LINK_UP)); + debug(" LINK_SPEED: %d\n", (val >> 16) & 0xF); + debug(" LINK_WIDTH: %d\n", (val >> 20) & 0x3F);
1021-1031: Duplicate header includes.
<asm/io.h>and<asm/gpio.h>are included twice (lines 1021-1022 and 1029-1030).🔧 Suggested fix
`#include` <asm/io.h> `#include` <asm/gpio.h> `#include` <clk.h> `#include` <reset.h> `#include` <generic-phy.h> `#include` <pci.h> `#include` <power-domain.h> `#include` <power/regulator.h> -#include <asm/io.h> -#include <asm/gpio.h> `#include` <errno.h>
1179-1181: More duplicate macro definitions.
PCIE_LINK_WIDTH_SPEED_CONTROLandPORT_LOGIC_SPEED_CHANGEare already defined at lines 1046-1047.🔧 Suggested fix
-#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C -#define PORT_LOGIC_SPEED_CHANGE (0x1 << 17) `#define` LINK_CONTROL2_LINK_STATUS2 0xa0patch/kernel/archive/sunxi-6.18/patches.armbian/drv-phy-allwinner-add-pcie-usb3-driver.patch (1)
1101-1104: Handlepm_runtime_get_sync()errors.Ignoring the return value can leave the device in an inconsistent PM state if runtime PM activation fails.
♻️ Suggested fix
- pm_runtime_get_sync(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + return ret; + }
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-phy-allwinner-add-pcie-usb3-driver.patch
Show resolved
Hide resolved
rvdr
force-pushed
the
spi-nvme-patches
branch
from
8a4ed2e to
bac187a
Compare
…g options that are not needed.
enable_extension "wayland-sessions-mask" instead
…y CE config options - also drop BOOT_SUPPORT_SPI & BOOT_SPI_RKSPI_LOADER as those are for vendor u-boot, not used here
Add loong64 to the list of architectures prepared by prepare_host_binfmt_qemu_cross(). This allows automatic registration and use of qemu-user emulation for LoongArch64 guests, enabling rootfs bootstrap and CI workflows targeting loong64. This aligns Armbian with Debian’s upcoming native loong64 support (Forky) and allows testing already via debian-ports and qemu-system-loongarch64. Signed-off-by: Igor Pecovnik <igor@armbian.com>
- mainline AT-F only for non-vendor BRANCH - vendor branch uses vendor u-boot with rkbin blobs - sorry for the confusion; I overlooked the conditional here - Fixes: f45765e
Correcting boot issues due to the iommu driver. Applying EvilOlaf's changes. Applying EvilOlaf's changes. Changing subjects and file names Resolving CodeRabbit detected issues. Reverting changes due to boot issues. Reverting changes due to boot issues.
rvdr
force-pushed
the
spi-nvme-patches
branch
from
bac187a to
a54bd97
Compare
rvdr
requested review from
a team,
hzyitc,
paolosabatino and
rpardini
as code owners
github-actions
bot
added
Framework
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch`:
- Around line 1525-1533: The two unbounded busy-wait loops after
sunxi_iommu_write of IOMMU_TLB_IVLD_ENABLE_REG and IOMMU_PC_IVLD_ENABLE_REG can
hang indefinitely in IRQ context; replace each plain while
(sunxi_iommu_read(...) & 0x1) loop with a bounded wait using the existing
sunxi_wait_when() helper (or an equivalent timeout loop) and handle timeout
failure (log/error return) instead of spinning forever while holding
IRQ/spinlock; update the code paths around sunxi_iommu_write/sunxi_iommu_read
and the IOMMU_TLB_IVLD_ENABLE_REG / IOMMU_PC_IVLD_ENABLE_REG checks to use that
timeout and early-fail behavior so IRQ context cannot be blocked indefinitely.
In
`@patch/u-boot/sunxi-dev-u-boot-a523/allwinner-a523-support-spi-controllers.patch`:
- Around line 40-59: The readl_poll_timeout call in sun4i_spi_xfer that waits
for FIFO reset uses a timeout value in microseconds but currently passes
SUN4I_SPI_TIMEOUT_MS (1000 ms) directly; update the call in the block guarded by
priv->variant->has_bsr to pass the timeout in microseconds (e.g.,
SUN4I_SPI_TIMEOUT_MS * 1000 or otherwise multiply the 1000-ms constant by 1000)
so the fourth argument to readl_poll_timeout(SPI_REG(priv, SPI_FCR), reg, !(reg
& (SPI_BIT(priv, SPI_FCR_RF_RST) | SPI_BIT(priv, SPI_FCR_TF_RST))), ...) is in
microseconds and not milliseconds.
♻️ Duplicate comments (17)
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch (5)
2094-2100: Clock error code is still lost due to ordering.
PTR_ERR(sunxi_iommu->clk)is called after settingclk = NULL, soretbecomes 0 instead of the actual error code. This allows probe to incorrectly fail withret = 0.🐛 Proposed fix
sunxi_iommu->clk = of_clk_get_by_name(dev->of_node, "iommu"); if (IS_ERR(sunxi_iommu->clk)) { + ret = PTR_ERR(sunxi_iommu->clk); sunxi_iommu->clk = NULL; dev_dbg(dev, "Unable to find clock\n"); - ret = PTR_ERR(sunxi_iommu->clk); /* Korrekter Fehlercode */ goto err_clk; }
2051-2059:iopte_cacheleaked ifdevm_kzallocfails.
sunxi_pgtable_alloc_pte_cache()allocatesiopte_cachebefore thedevm_kzalloc. If the allocation fails, the cache is never freed.🐛 Proposed fix
sunxi_iommu = devm_kzalloc(dev, sizeof(*sunxi_iommu), GFP_KERNEL); - if (!sunxi_iommu) + if (!sunxi_iommu) { + sunxi_pgtable_free_pte_cache(iopte_cache); return -ENOMEM; + }
1430-1441: Validateargs_countbefore accessingargs[1].The code reads
args->args[1]without checking ifargs->args_count > 1. If the device tree binding specifies only one cell (#iommu-cells = <1>), this is an out-of-bounds read.🐛 Proposed fix
if (!owner) { owner = kzalloc(sizeof(*owner), GFP_KERNEL); if (!owner) { put_device(&sysmmu->dev); return -ENOMEM; } owner->tlbid = args->args[0]; - owner->flag = args->args[1]; + owner->flag = (args->args_count > 1) ? args->args[1] : 0; owner->data = data; owner->dev = dev; dev_iommu_priv_set(dev, owner); }
2208-2213: Suspend uses global variable instead of device context.
sun55i_iommu_suspend()accessesglobal_iommu_devwithout NULL check and ignores thedevparameter, whilesun55i_iommu_resume()correctly usesdev_get_drvdata(dev).🔧 Proposed fix
static int sun55i_iommu_suspend(struct device *dev) { - clk_disable_unprepare(global_iommu_dev->clk); + struct sunxi_iommu_dev *iommu = dev_get_drvdata(dev); + + if (iommu && iommu->clk) + clk_disable_unprepare(iommu->clk); return 0; }
2081-2175: German inline comments should be translated to English or removed.Multiple lines contain German comments (e.g., "Korrekter Fehlercode", "Fehlerbehandlung", "devm_* ressourcenverwaltet ist"). Per kernel coding style and previous review discussion, these should be in English.
patch/u-boot/sunxi-dev-u-boot-a523/clk-sunxi-fix-clock-handling-for-sun55i-a523.patch (1)
22-23: Add the missingCLK_PCIE_AUXbinding constant.
CLK_PCIE_AUXis used inclk_a523.cbut isn’t defined in the dt‑bindings header, which will fail compilation. Please add it alongsideCLK_USB3_REF.🐛 Proposed fix
`#define` CLK_FANOUT0 176 `#define` CLK_FANOUT1 177 `#define` CLK_FANOUT2 178 `#define` CLK_USB3_REF 179 +#define CLK_PCIE_AUX 180Also applies to: 51-52
patch/u-boot/sunxi-dev-u-boot-a523/arm64-dts-sun55i-t527-orangepi-4a-enable-spi0-pcie-combophy.patch (1)
62-68: Definereg_pcie_vcc3v3forslot-3v3-supply.
slot-3v3-supply = <®_pcie_vcc3v3>references a phandle that isn’t defined in the U-Boot DTS. This will break DT compilation or leave the supply unresolved. Please add a fixed regulator node (or reference an existing 3.3 V rail) in the U-Boot DTS tree.patch/kernel/archive/sunxi-6.18/patches.armbian/arm64-dts-sun55i-dtsi-add-iommu-usbc-pcie-combophy-nodes.patch (1)
45-115: Ensureusbc1,combophy, andpcieare children ofsoc.If these nodes end up at the root instead of under
soc, they won’t inherit thesimple-busranges and address decoding can break. Please confirm the final DTS tree places them undersoc(same level asppu).patch/kernel/archive/sunxi-6.18/patches.armbian/drv-phy-allwinner-add-pcie-usb3-driver.patch (4)
203-228: Fix mode/reset bit handling incombo_phy_mode_set.
tmpstarts at 0, so bit updates don’t apply to the live register; the disable path can leave stale mode/reset state. Use the current register value as the base.🐛 Suggested fix
- u32 val, tmp = 0; - val = readl(COMBO_REG_PHYCTRL(combphy->phy_ctl)); - - if (combphy->user == PHY_USE_BY_PCIE) - tmp &= ~PHY_USE_SEL; - else if (combphy->user == PHY_USE_BY_USB3) - tmp |= PHY_USE_SEL; - else if (combphy->user == PHY_USE_BY_PCIE_USB3_U2) - tmp &= ~PHY_USE_SEL; - - if (combphy->ref == INTER_SIG_REF_CLK) - tmp &= ~PHY_CLK_SEL; - else if (combphy->ref == EXTER_DIF_REF_CLK) - tmp |= PHY_CLK_SEL; - - if (enable) { - tmp |= PHY_RSTN; - val |= tmp; - } else { - tmp &= ~PHY_RSTN; - val &= ~tmp; - } - writel(val, COMBO_REG_PHYCTRL(combphy->phy_ctl)); + u32 val; + val = readl(COMBO_REG_PHYCTRL(combphy->phy_ctl)); + + val &= ~PHY_USE_SEL; + if (combphy->user == PHY_USE_BY_USB3) + val |= PHY_USE_SEL; + + val &= ~PHY_CLK_SEL; + if (combphy->ref == EXTER_DIF_REF_CLK) + val |= PHY_CLK_SEL; + + if (enable) + val |= PHY_RSTN; + else + val &= ~PHY_RSTN; + + writel(val, COMBO_REG_PHYCTRL(combphy->phy_ctl));
309-339: Guardclk_set_rate()against ERR_PTR clocks.
phyclk_ref/phyclk_cfgare logged but can still be ERR_PTR. Callingclk_set_rate()on an error pointer can crash. Either skipclk_set_rate()when ERR_PTR or treat the clock as required and returnPTR_ERR().🐛 Minimal guard (skip if ERR_PTR)
- if (!already_enabled && combphy->user != PHY_USE_BY_USB3) { + if (!already_enabled && combphy->user != PHY_USE_BY_USB3 && + !IS_ERR(combphy->phyclk_ref)) { ret = clk_set_rate(combphy->phyclk_ref, 100000000); if (ret) dev_err(dev, "failed to set phyclk_ref freq 100M!\n"); } @@ - if (combphy->drvdata->has_cfg_clk) { + if (combphy->drvdata->has_cfg_clk) { if (!already_enabled) { - ret = clk_set_rate(combphy->phyclk_cfg, 200000000); + if (!IS_ERR(combphy->phyclk_cfg)) + ret = clk_set_rate(combphy->phyclk_cfg, 200000000); if (ret) dev_err(dev, "failed to set phyclk_cfg freq 200M!\n"); }
863-878: Propagate USB3 power-on errors.
sunxi_combphy_usb3_power_on()can fail (regulator ops), but its return value is ignored. This masks failures from the PHY framework.🐛 Suggested fix
case PHY_TYPE_USB3: - sunxi_combphy_usb3_power_on(combphy); + ret = sunxi_combphy_usb3_power_on(combphy); + if (ret) + return ret; break;
953-980: Guardclk_set_parent()against ERR_PTR inputs.
refclk_par/cfgclk_parfailures are only logged, but still passed toclk_set_parent(). If either is ERR_PTR, this will crash.🐛 Suggested guard
combphy->refclk_par = devm_clk_get(&pdev->dev, "refclk_par"); if (IS_ERR(combphy->refclk_par)) - dev_dbg(dev, "failed to get refclk_par\n"); + return PTR_ERR(combphy->refclk_par); ret = clk_set_parent(combphy->phyclk_ref, combphy->refclk_par);patch/u-boot/sunxi-dev-u-boot-a523/arm64-dts-sun55i-a527-cubie-a5e-enable-spi0-pcie-combophy.patch (1)
93-99: Usepinsinstead ofallwinner,pinsforpcie_pwren_pins.The other pinctrl nodes in this patch use the generic
pinsproperty. If U-Boot’s pinctrl expectspins,allwinner,pinsmay be ignored, leaving the GPIO unconfigured. Please verify and switch topinsif needed.♻️ Suggested tweak
- allwinner,pins = "PL11"; + pins = "PL11";patch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch (4)
368-377: PHY cleanup incomplete on power_on failure.If
generic_phy_power_on()fails at line 373-377, the error path jumps toerr_assert_reset, butgeneric_phy_init()(line 368) has already succeeded. The PHY should be de-initialized viageneric_phy_exit()before proceeding with reset assertion.🛠️ Suggested fix
ret = generic_phy_power_on(&pci->phy); if (ret) { printf("PCIe: Failed to power on phy: %d\n", ret); - goto err_assert_reset; + goto err_exit_phy; } printf("PCIe: Toggling external device reset (PERST#)...\n"); @@ -399,6 +399,8 @@ static int sun55i_pcie_plat_init_port(struct udevice *dev) err_power_off_phy: generic_phy_power_off(&pci->phy); +err_exit_phy: + generic_phy_exit(&pci->phy); err_assert_reset: if (pci->drvdata && pci->drvdata->need_pcie_rst) reset_assert(&pci->pcie_rst);
682-687: Missing indentation forsize_len = 2.Line 685 is missing the tab indentation for the
size_len = 2;statement inside theelse ifblock.🔧 Suggested fix
if (size == PCI_SIZE_8) size_len = 1; else if (size == PCI_SIZE_16) - size_len = 2; + size_len = 2; else if (size == PCI_SIZE_32) size_len = 4;
847-850: LNKCTL2 access uses hardcoded offset instead of discovered capability base.The code discovers the PCIe capability offset at line 840, but lines 847-850 still use the hardcoded
LINK_CONTROL2_LINK_STATUS2(0xa0) instead ofoffset + PCI_EXP_LNKCTL2. If the PCIe capability isn't at the standard 0x70 offset, this will access the wrong register.🐛 Proposed fix
- val = sun55i_pcie_readl_dbi(pci, LINK_CONTROL2_LINK_STATUS2); + val = sun55i_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2); val &= ~PCI_EXP_LNKCTL2_TLS; val |= gen; - sun55i_pcie_writel_dbi(pci, LINK_CONTROL2_LINK_STATUS2, val); + sun55i_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, val);
1088-1096: Duplicate macro definitions will cause preprocessor warnings.The following macros are defined twice:
PCIE_ATU_TYPE_MEM,PCIE_ATU_TYPE_IO,PCIE_ATU_TYPE_CFG0,PCIE_ATU_TYPE_CFG1(lines 1072-1075 and 1088-1091)PCIE_ATU_FUNC_NUM(lines 1092 and 1096)🔧 Suggested fix
Remove the duplicate definitions at lines 1088-1091 and 1096:
`#define` PCIE_ATU_CR1_INBOUND(reg) (0x300100 + ((reg) * 0x200)) -#define PCIE_ATU_TYPE_MEM (0x0 << 0) -#define PCIE_ATU_TYPE_IO (0x2 << 0) -#define PCIE_ATU_TYPE_CFG0 (0x4 << 0) -#define PCIE_ATU_TYPE_CFG1 (0x5 << 0) `#define` PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) `#define` PCIE_ATU_CR2_INBOUND(reg) (0x300104 + ((reg) * 0x200)) `#define` PCIE_ATU_MATCH_MODE BIT(30) `#define` PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) -#define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
🧹 Nitpick comments (6)
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch (2)
26-34: Kconfig uses spaces instead of tabs for indentation.Kernel Kconfig files require tab indentation per kernel coding style. Lines 27-34 appear to use spaces.
♻️ Suggested fix
config SUN55I_IOMMU - bool "Allwinner A523 IOMMU Support" - depends on HAS_DMA - depends on ARCH_SUNXI || COMPILE_TEST - select ARM_DMA_USE_IOMMU - select IOMMU_API - select IOMMU_DMA - help - Support for the IOMMU introduced in the Allwinner A523 SoCs. + bool "Allwinner A523 IOMMU Support" + depends on HAS_DMA + depends on ARCH_SUNXI || COMPILE_TEST + select ARM_DMA_USE_IOMMU + select IOMMU_API + select IOMMU_DMA + help + Support for the IOMMU introduced in the Allwinner A523 SoCs.
46-48: Trailing whitespace on line 46.Remove trailing space after
sunxi-iommu.o.♻️ Suggested fix
-obj-$(CONFIG_SUN55I_IOMMU) += sunxi-iommu.o +obj-$(CONFIG_SUN55I_IOMMU) += sunxi-iommu.opatch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch (4)
26-32: Minor: Trailing whitespace in Kconfig help text.Line 32 has trailing whitespace after the help text. While functional, this may trigger checkpatch warnings.
+config PCIE_SUN55I_RC + bool "Allwinner SUN55I DesignWare PCIe controller" + default n + depends on ARCH_SUNXI + help - Enables support for the DW PCIe controller in the Allwinner Sun55i SoC. - + Enables support for the DW PCIe controller in the Allwinner Sun55i SoC.
96-118: Inconsistent indentation (spaces vs tabs).This function uses space indentation while the rest of the file uses tabs. This also affects other sections (lines 333-338, 449-458).
1021-1031: Duplicate includes.
<asm/io.h>and<asm/gpio.h>are included twice (lines 1021-1022 and 1029-1030). While harmless due to include guards, this indicates copy-paste that should be cleaned up.
1046-1047: Duplicate macro:PCIE_LINK_WIDTH_SPEED_CONTROL.This macro is defined at both line 1046 and line 1179 with the same value (0x80C). Remove one of the definitions.
Also applies to: 1179-1181
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch
Show resolved
Hide resolved
patch/u-boot/sunxi-dev-u-boot-a523/allwinner-a523-support-spi-controllers.patch
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@patch/u-boot/sunxi-dev-u-boot-a523/clk-sunxi-fix-clock-handling-for-sun55i-a523.patch`:
- Line 35: Add a missing dt-binding define for RST_BUS_PCIE_USB3: edit the
dt-bindings header where CLK_USB3_REF is defined and add a `#define`
RST_BUS_PCIE_USB3 with the same numeric ID used by the drivers mapping (i.e.,
the value corresponding to the new resets entry in drivers/clk/sunxi/clk_a523.c
which maps RESET(0xaac, BIT(24))). Place this new define immediately alongside
CLK_USB3_REF so device tree files can reference RST_BUS_PCIE_USB3 in their
resets properties.
In `@patch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch`:
- Around line 642-693: In sun55i_pcie_read_config and sun55i_pcie_write_config
replace the hard-coded root-bus check "PCI_BUS(bdf) != 0" with a comparison
against the controller's actual root bus number (pcie->first_busno) so root
config accesses use host functions when PCI_BUS(bdf) == pcie->first_busno;
update both functions (sun55i_pcie_read_config and sun55i_pcie_write_config) to
use "PCI_BUS(bdf) != pcie->first_busno" (or the inverse) when deciding between
sun55i_pcie_rd_other_conf / sun55i_pcie_host_rd_own_conf and
sun55i_pcie_wr_other_conf / sun55i_pcie_host_wr_own_conf.
♻️ Duplicate comments (15)
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-iommu-sunxi-add-iommu-driver.patch (5)
2032-2032: German comments should be in English.Multiple German comments remain in the probe function (lines 2032, 2046, 2058, 2075, 2089, 2113, 2121, 2127, 2135, 2156). Kernel/driver code should use English comments. Either translate or remove these developer notes:
- "Es gab einen doppelten Check" → remove or translate
- "Korrekte Fehlerprüfung" → remove
- "KORREKTUR 1-4" → remove (code is now correct)
- etc.
1417-1418: Out-of-bounds read:args->args[1]when#iommu-cells = <1>.The device tree binding uses
#iommu-cells = <2>(line 35 shows this), but if clients use only one cell, accessingargs->args[1]without checkingargs->args_countcauses an OOB read. Add validation:if (!owner) { owner = kzalloc(sizeof(*owner), GFP_KERNEL); if (!owner) return -ENOMEM; owner->tlbid = args->args[0]; - owner->flag = args->args[1]; + owner->flag = (args->args_count > 1) ? args->args[1] : 0;
2071-2077: PTR_ERR called after pointer set to NULL returns 0.The error handling captures the error code after setting the pointer to NULL, which makes
PTR_ERR()return 0 instead of the actual error. Reorder the statements:sunxi_iommu->clk = of_clk_get_by_name(dev->of_node, "iommu"); if (IS_ERR(sunxi_iommu->clk)) { + ret = PTR_ERR(sunxi_iommu->clk); sunxi_iommu->clk = NULL; dev_dbg(dev, "Unable to find clock\n"); - ret = PTR_ERR(sunxi_iommu->clk); goto err_clk; }
2188-2193: Suspend uses global variable instead of device context.
sun55i_iommu_suspend()usesglobal_iommu_devwithout NULL check and ignores thedevparameter, whilesun55i_iommu_resume()correctly usesdev_get_drvdata(dev). For consistency and safety:static int sun55i_iommu_suspend(struct device *dev) { - clk_disable_unprepare(global_iommu_dev->clk); + struct sunxi_iommu_dev *iommu = dev_get_drvdata(dev); + + if (iommu && iommu->clk) + clk_disable_unprepare(iommu->clk); return 0; }
1505-1513: Unbounded busy-wait in IRQ handler can cause system hang.These
whileloops wait for hardware bits to clear without any timeout. If hardware malfunctions, this will hang indefinitely while holding a spinlock in IRQ context. Consider usingsunxi_wait_when()macro (already defined in this file) or documenting why unbounded wait is acceptable.- while (sunxi_iommu_read(iommu, IOMMU_TLB_IVLD_ENABLE_REG) & 0x1) - ; + sunxi_wait_when(sunxi_iommu_read(iommu, IOMMU_TLB_IVLD_ENABLE_REG) & 0x1, 2); ... - while (sunxi_iommu_read(iommu, IOMMU_PC_IVLD_ENABLE_REG) & 0x1) - ; + sunxi_wait_when(sunxi_iommu_read(iommu, IOMMU_PC_IVLD_ENABLE_REG) & 0x1, 2);patch/u-boot/sunxi-dev-u-boot-a523/clk-sunxi-fix-clock-handling-for-sun55i-a523.patch (1)
22-23: MissingCLK_PCIE_AUXdefinition in dt-bindings header.The patch adds
CLK_PCIE_AUXto the gates array but only definesCLK_USB3_REF(value 179) in the dt-bindings header.CLK_PCIE_AUXneeds a corresponding#defineinsun55i-a523-ccu.hto avoid an undefined identifier error.🐛 Proposed fix
Add the missing definition in the dt-bindings header section:
`#define` CLK_FANOUT2 178 `#define` CLK_USB3_REF 179 +#define CLK_PCIE_AUX 180 `#endif` /* _DT_BINDINGS_CLK_SUN55I_A523_CCU_H_ */patch/kernel/archive/sunxi-6.18/patches.armbian/arm64-dts-sun55i-dtsi-add-iommu-usbc-pcie-combophy-nodes.patch (2)
45-116: Nodesusbc1,combophy, andpcieappear to be outside thesoc {}block.These memory-mapped peripheral nodes (lines 45-115) lack the proper indentation to place them inside the
soc {}block. Compare with theppunode at line 117 which has proper tab indentation as a child ofsoc. All these peripherals should be children of thesocnode.🔧 Suggested fix
Add one level of tab indentation to all these nodes and their contents:
-usbc1: usbc1@11 { - device_type = "usbc1"; + usbc1: usbc1@11 { + device_type = "usbc1"; ...
27-36: Moveiommu@2010000inside thesoc {}block and fix indentation ofusbc1,combophy, andpcienodes.The IOMMU node should be a child of the SoC bus node (inside
soc {}), following the pattern used in sun50i-h616 and sun20i-d1. Currently it is incorrectly placed at the root level.Additionally, the
usbc1,combophy, andpcienodes (lines ~859–931) have zero indentation and appear to be insidesoc {}but lack proper tab indentation. Verify their correct placement and apply consistent indentation.patch/u-boot/sunxi-dev-u-boot-a523/allwinner-a523-support-spi-controllers.patch (1)
50-53: Timeout value in wrong units - should be microseconds.
readl_poll_timeout()expects the timeout parameter in microseconds, but1000is passed directly. If intent is 1000ms timeout, it should be1000 * 1000(orSUN4I_SPI_TIMEOUT_MS * 1000). Currently this only waits 1ms.ret = readl_poll_timeout(SPI_REG(priv, SPI_FCR), reg, !(reg & (SPI_BIT(priv, SPI_FCR_RF_RST) | SPI_BIT(priv, SPI_FCR_TF_RST))), - 1000); + SUN4I_SPI_TIMEOUT_MS * 1000);patch/u-boot/sunxi-dev-u-boot-a523/arm64-dts-sun55i-t527-orangepi-4a-enable-spi0-pcie-combophy.patch (1)
44-68: Verifyreg_pcie_vcc3v3exists in the U‑Boot DTS tree.
If the label is missing in U‑Boot (even if present in the kernel DTS), this will fail dtc.#!/bin/bash # Verify the reg_pcie_vcc3v3 label is defined somewhere in U-Boot DTS patches rg -n "reg_pcie_vcc3v3:" patch/u-boot rg -n "slot-3v3-supply\s*=\s*<®_pcie_vcc3v3>" patch/u-bootpatch/u-boot/sunxi-dev-u-boot-a523/arm64-dts-sun55i-a527-cubie-a5e-enable-spi0-pcie-combophy.patch (1)
93-99: Consider usingpinsinstead ofallwinner,pinsfor consistency with the other pinctrl nodes.
This patch usespinselsewhere; aligning the property name avoids mixed styles and better matches modern bindings.♻️ Suggested tweak
- allwinner,pins = "PL11"; + pins = "PL11";patch/u-boot/sunxi-dev-u-boot-a523/pci-sunxi-enable-pcie-support.patch (4)
368-377: Power‑on failure should undo PHY enable.
At Line 373‑376, ageneric_phy_power_on()failure jumps toerr_assert_reset, skippinggeneric_phy_power_off(). Use the existingerr_power_off_phylabel to avoid leaving the PHY partially powered.🐛 Proposed fix
ret = generic_phy_power_on(&pci->phy); if (ret) { printf("PCIe: Failed to power on phy: %d\n", ret); - goto err_assert_reset; + goto err_power_off_phy; }
578-599: CFG0/CFG1 selection should distinguish the first downstream bus.
At Line 589‑615, CFG0 is chosen for any non‑zero bus, which breaks config access behind a switch. Use CFG0 only for the first downstream bus and CFG1 for others.🐛 Proposed fix
static int sun55i_pcie_rd_other_conf(struct sun55i_pcie_port *pp, pci_dev_t d, int where, int size, ulong *val) { int ret = PCIBIOS_SUCCESSFUL, type; + struct sun55i_pcie *pcie = to_sun55i_pcie_from_pp(pp); u64 busdev; u64 atu_cpu_addr = pp->cfg0_base; @@ - if (PCI_BUS(d) != 0) + if (PCI_BUS(d) == pcie->first_busno + 1) type = PCIE_ATU_TYPE_CFG0; else type = PCIE_ATU_TYPE_CFG1; @@ static int sun55i_pcie_wr_other_conf(struct sun55i_pcie_port *pp, pci_dev_t d, int where, int size, ulong val) { int ret = PCIBIOS_SUCCESSFUL, type; + struct sun55i_pcie *pcie = to_sun55i_pcie_from_pp(pp); u64 busdev; u64 atu_cpu_addr = pp->cfg0_base; @@ - if (PCI_BUS(d) != 0) + if (PCI_BUS(d) == pcie->first_busno + 1) type = PCIE_ATU_TYPE_CFG0; else type = PCIE_ATU_TYPE_CFG1;Also applies to: 601-616
832-858: Use PCIe capability base for LNKCTL2 writes.
At Line 847‑851,offsetis computed but the code reads/writesLINK_CONTROL2_LINK_STATUS2directly. If the PCIe capability isn’t at the default offset, the wrong register is updated.🐛 Proposed fix
- val = sun55i_pcie_readl_dbi(pci, LINK_CONTROL2_LINK_STATUS2); + val = sun55i_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2); val &= ~PCI_EXP_LNKCTL2_TLS; val |= gen; - sun55i_pcie_writel_dbi(pci, LINK_CONTROL2_LINK_STATUS2, val); + sun55i_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, val);
1071-1096: Remove duplicate ATU macro definitions.
At Line 1088‑1096,PCIE_ATU_TYPE_*andPCIE_ATU_FUNC_NUMare re‑defined, which can trigger macro‑redefinition warnings.🧹 Proposed fix
`#define` PCIE_ATU_CR1_INBOUND(reg) (0x300100 + ((reg) * 0x200)) -#define PCIE_ATU_TYPE_MEM (0x0 << 0) -#define PCIE_ATU_TYPE_IO (0x2 << 0) -#define PCIE_ATU_TYPE_CFG0 (0x4 << 0) -#define PCIE_ATU_TYPE_CFG1 (0x5 << 0) `#define` PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) `#define` PCIE_ATU_CR2_INBOUND(reg) (0x300104 + ((reg) * 0x200)) `#define` PCIE_ATU_MATCH_MODE BIT(30) `#define` PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) -#define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20)
🧹 Nitpick comments (3)
patch/kernel/archive/rockchip-6.18/series.conf (1)
63-64: Add rationale for disabling these patches.These patches touch VOP2 polling and HDMI 2.1 FRL; commenting them out may regress RK3588 display/HDMI behavior. Consider adding a short inline comment with the reason (e.g., build failure, runtime regression, upstream conflict) and a reference (issue/commit) to avoid accidental re‑enablement later.
💡 Suggested inline rationale comment
-# patches.libreelec/rockchip-0062-FROMLIST-v1-drm-rockchip-vop2-Add-delay-between-poll.patch -# patches.libreelec/rockchip-0063-FROMLIST-v1-drm-rockchip-vop2-Only-wait-for-changed-.patch +# Disabled: causes regression in RK3588 VOP2 polling (see issue/commit XYZ) +# patches.libreelec/rockchip-0062-FROMLIST-v1-drm-rockchip-vop2-Add-delay-between-poll.patch +# patches.libreelec/rockchip-0063-FROMLIST-v1-drm-rockchip-vop2-Only-wait-for-changed-.patch ... -# patches.libreelec/rockchip-0105-WIP-FRL-drm-bridge-dw-hdmi-qp-Add-HDMI-2.1-FRL-suppo.patch +# Disabled: HDMI 2.1 FRL WIP series conflicts with 6.18 base (see issue/commit XYZ) +# patches.libreelec/rockchip-0105-WIP-FRL-drm-bridge-dw-hdmi-qp-Add-HDMI-2.1-FRL-suppo.patchAlso applies to: 106-106
extensions/wayland-sessions-mask.sh (1)
1-1: Consider dropping the shebang in extension scripts.These files are sourced by the build system, so the shebang isn’t used; removing it keeps conventions consistent and avoids suggesting direct execution.
♻️ Suggested cleanup
-#!/usr/bin/env bashBased on learnings, extensions are sourced and don’t need a shebang.
patch/u-boot/sunxi-dev-u-boot-a523/allwinner-a523-support-spl-spi-controllers.patch (1)
125-196: Inconsistent indentation: spaces instead of tabs.The function body uses 4-space indentation instead of tabs, which is inconsistent with the kernel/U-Boot coding style and the rest of the file (visible in the context lines). This affects lines 125-196.
🔧 Suggested fix
Convert all 4-space indentations to single tabs throughout this function to maintain consistency with U-Boot coding style.
patch/u-boot/sunxi-dev-u-boot-a523/clk-sunxi-fix-clock-handling-for-sun55i-a523.patch
Show resolved
Hide resolved
7 participants
Description
Summary: These changes add support for PCIe, SPI-Flash and Iommu on the Radxa Cubie A5E and Orange Pi 4a. The changes for the Pi 4a couldnt be tested due to a lack of hardware.
These are the build logs:
https://paste.armbian.eu/vesojokuva
Documentation summary for feature / change
[x] short description: Add PCIe, USB3-PHY, and IOMMU support for sun55i boards.
[x] summary: Users of Cubie A5E and Orange Pi 4A can now use PCIe/NVMe and USB3.0 stable.
[x] example of usage: Booting from NVMe or using lspci to list connected PCIe devices.
How Has This Been Tested?
[x] Test A: Flashing the bootloader to an SD-Card and Booting from the NVMe. No Issues after 24 hours of runtime.
[x] Test B: Booting directly from the SD-Card. SPI-Flash and NVMe SSD are getting detected.
Checklist:
[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] My changes generate no new warnings
Summary by CodeRabbit
New Features
Configuration Changes
✏️ Tip: You can customize this high-level summary in your review settings.