Re: [PATCH v5 3/6] iommu/arm-smmu-v3: Add feature detection for HTTU

[Nicolin Chen <nicolinc@nvidia.com>]

Hi Shameer,

Some nitpicking inline.

On Thu, Jun 06, 2024 at 02:32:59PM +0100, Shameer Kolothum wrote:

> 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 e30ce55ed1af..c05a74aa52a4 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3738,6 +3738,29 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu)
>         }
>  }
> 
> +static void arm_smmu_get_httu(struct arm_smmu_device *smmu, u32 reg)

Following the other helper arm_smmu_device_iidr_probe, how about
arm_smmu_device_httu_probe?

And we could pass in FIELD_GET(IDR0_HTTU, reg) too?

> +{
> +       u32 fw_features = smmu->features & (ARM_SMMU_FEAT_HA | ARM_SMMU_FEAT_HD);
> +       u32 httu = FIELD_GET(IDR0_HTTU, reg);
> +       u32 features = 0;

How about "hw_feats" v.s. "fw_feats"?

> +
> +       switch (httu) {
> +       case IDR0_HTTU_ACCESS_DIRTY:
> +               features |= ARM_SMMU_FEAT_HD;
> +               fallthrough;
> +       case IDR0_HTTU_ACCESS:
> +               features |= ARM_SMMU_FEAT_HA;
> +       }
> +
> +       if (smmu->dev->of_node)
> +               smmu->features |= features;
> +       else if (features != fw_features)
> +               /* ACPI IORT sets the HTTU bits */
> +               dev_warn(smmu->dev,
> +                        "IDR0.HTTU(0x%x) overridden by FW configuration (0x%x)\n",
> +                         httu, fw_features);

httu and fw_features have different shifts -- could be odd to see:
	IDR0.HTTU(0x2) overridden by FW configuration (0x600000)

FIELD_GET(ACPI_IORT_SMMU_V3_HTTU_OVERRIDE, iort_smmu->flags) seems
to be overcomplicated to reference? So, how about just features?
+			"IDR0.HTTU features (0x%x) overridden by FW configuration (0x%x)\n",

Thanks
Nicolin