Re: [PATCH v7 08/11] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, May 27, 2026 at 10:14:04PM +0000, Pranjal Shrivastava wrote:
> +/* Runtime PM helpers */
> +__maybe_unused static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +
> +	if (pm_runtime_enabled(smmu->dev)) {
> +		ret = pm_runtime_resume_and_get(smmu->dev);
> +		if (ret < 0) {
> +			dev_err(smmu->dev, "failed to resume device: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;

Nit: ret is used within the first if.

Yet, I wouldn't like it to move. So, maybe do an early return:

	if (!pm_runtime_enabled(smmu->dev))
		return 0;

> +__maybe_unused static void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +
> +	if (pm_runtime_enabled(smmu->dev)) {
> +		ret = pm_runtime_put_autosuspend(smmu->dev);
> +		if (ret < 0)
> +			dev_err(smmu->dev, "failed to suspend device: %d\n", ret);
> +	}
> +}

Ditto

> +
> +static inline u32 arm_smmu_cmdq_owner_prod_idx(struct arm_smmu_cmdq *cmdq)
> +{
> +	return atomic_read(&cmdq->owner_prod) & CMDQ_PROD_IDX_MASK;
> +}
> +
>  static void parse_driver_options(struct arm_smmu_device *smmu)
>  {
>  	int i = 0;
> @@ -789,7 +822,8 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
>  		/* b. Stop gathering work by clearing the owned flag */
>  		prod = atomic_fetch_andnot_relaxed(CMDQ_PROD_OWNED_FLAG,
>  						   &cmdq->q.llq.atomic.prod);
> -		prod &= ~CMDQ_PROD_OWNED_FLAG;
> +		/* Strip all metadata flags */
> +		prod &= CMDQ_PROD_IDX_MASK;

Should its prior atomic_fetch_andnot_relaxed() call do something
about the CMDQ_PROD_STOP_FLAG as well?

> +/*
> + * Lockless pre-check to elide invalidations if SMMU is suspended.
> + * Races with concurrent suspend are benign: the cmpxchg loop in
> + * arm_smmu_cmdq_issue_cmdlist() acts as the true commit point.
> + * If we lose the race, that loop observes Q_STOP == 1 and safely
> + * drops the command. If we win, the suspend thread waits for us.
> + */
> +static inline bool arm_smmu_can_elide(struct arm_smmu_device *smmu)
> +{
> +	return !!Q_STOP(READ_ONCE(smmu->cmdq.q.llq.prod));
> +}

arm_smmu_cmdq_can_elide()

Should it handle a secondary_cmdq?

Nicolin