Re: [PATCH 3/5] iommu/arm-smmu-v3: enable optional clocks

From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: aiyengar1990@gmail.com
Cc: Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	 "Joerg Roedel (AMD)" <joro@8bytes.org>,
	Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	 Heiko Stuebner <heiko@sntech.de>,
	Niklas Cassel <cassel@kernel.org>,
	 linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	devicetree@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-rockchip@lists.infradead.org
Subject: Re: [PATCH 3/5] iommu/arm-smmu-v3: enable optional clocks
Date: Tue, 10 Nov 2026 15:53:28 +0100	[thread overview]
Message-ID: <avMvinXkzBpaRh5K@venus> (raw)
In-Reply-To: <20261110-smmu-v3-pcie-pd-v1-3-628819ca44b9@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

Hi,

On Tue, Nov 10, 2026 at 07:11:50AM +0000, Arun Iyengar via B4 Relay wrote:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> On some platforms the SMMU's clock is controlled by the OS, and without a
> consumer the clock framework gates it as unused late in boot. Any later
> register access, for example the CR0 write in the shutdown path, then
> stalls the interconnect. This happens on the RK3588 when no PCIe
> controller keeps the PCIe MMU-600 clock running.
> 
> Get and enable all clocks described in the firmware node before the
> registers are first accessed, and keep them enabled while the driver is
> bound. Nodes without clocks are not affected.
> 
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 6d4520c47308..cce66db93219 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -12,6 +12,7 @@
>  #include <linux/acpi.h>
>  #include <linux/acpi_iort.h>
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/crash_dump.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
> @@ -5786,6 +5787,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	resource_size_t ioaddr;
>  	struct arm_smmu_device *smmu;
>  	struct device *dev = &pdev->dev;
> +	struct clk_bulk_data *clks;
>  
>  	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
>  	if (!smmu)
> @@ -5800,6 +5802,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/* Optional clocks: keep them running for as long as we are bound */

useless comment, otherwise

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to enable clocks\n");
> +
>  	smmu = arm_smmu_impl_probe(smmu);
>  	if (IS_ERR(smmu))
>  		return PTR_ERR(smmu);
> 
> -- 
> 2.55.0
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]