Hello Jiaxing,
On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> rk_iommu only enabled a fixed {aclk,iface} pair. On the RK3576 NPU the
> MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
> dropped until those clocks run too (reads work, writes need more clocks).
> Take every clock described in the DT for the IOMMU instead of a fixed
> pair, so a platform can list the full set it needs.
>
> Signed-off-by: Jiaxing Hu <[email protected]>
> ---
> drivers/iommu/rockchip-iommu.c | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index ce586919b..49c841095 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -93,11 +93,6 @@ struct rk_iommu_domain {
> struct iommu_domain domain;
> };
>
> -/* list of clocks required by IOMMU */
> -static const char * const rk_iommu_clocks[] = {
> - "aclk", "iface",
> -};
> -
> struct rk_iommu_ops {
> phys_addr_t (*pt_address)(u32 dte);
> u32 (*mk_dtentries)(dma_addr_t pt_dma);
> @@ -1274,25 +1269,20 @@ static int rk_iommu_probe(struct platform_device
> *pdev)
> iommu->reset_disabled = device_property_read_bool(dev,
> "rockchip,disable-mmu-reset");
>
> - iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
> - iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
> - sizeof(*iommu->clocks), GFP_KERNEL);
> - if (!iommu->clocks)
> - return -ENOMEM;
> -
> - for (i = 0; i < iommu->num_clocks; ++i)
> - iommu->clocks[i].id = rk_iommu_clocks[i];
> -
> /*
> - * iommu clocks should be present for all new devices and devicetrees
> - * but there are older devicetrees without clocks out in the wild.
> - * So clocks as optional for the time being.
> + * Take every clock the devicetree provides. Most IOMMU instances
> + * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
> + * behind additional gates (CBUF/DSU) whose clocks must be running
> + * for register writes to land. Clocks stay optional because there
> + * are older devicetrees without clocks out in the wild.
> */
It does not seem necessary to modify the comments, as there are indeed
many MMUs with multiple clocks :)
> - err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
> - if (err == -ENOENT)
> + err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
> + if (err == -ENOENT || err == 0)
> iommu->num_clocks = 0;
err == 0 is redundant.
> - else if (err)
> + else if (err < 0)
> return err;
> + else
> + iommu->num_clocks = err;
>
> err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
> if (err)
--
Best,
Chaoyi
