Re: [PATCH v3 2/9] iommu/arm-smmu-v3: Add arm_smmu_strtab_l1/2_idx()

[Will Deacon <will@kernel.org>]

On Tue, Aug 06, 2024 at 08:31:16PM -0300, Jason Gunthorpe wrote:
> Don't open code the calculations of the indexes for each level, provide
> two functions to do that math and call them in all the places. Update all
> the places computing indexes.
> 
> Calculate the L1 table size directly based on the max required index from
> the cap. Remove STRTAB_L1_SZ_SHIFT in favour of STRTAB_NUM_L2_STES.
> 
> Use STRTAB_NUM_L2_STES to replace remaining open coded 1 << STRTAB_SPLIT.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 51 +++++++++------------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 14 +++++-
>  2 files changed, 35 insertions(+), 30 deletions(-)
> 
> 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 b80f3359a8d12b..a695e5f8fc2880 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1670,20 +1670,17 @@ static void arm_smmu_init_initial_stes(struct arm_smmu_ste *strtab,
>  
>  static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>  {
> -	size_t size;
> -	void *strtab;
>  	dma_addr_t l2ptr_dma;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> -	struct arm_smmu_strtab_l1_desc *desc = &cfg->l1_desc[sid >> STRTAB_SPLIT];
> +	struct arm_smmu_strtab_l1_desc *desc =
> +		&cfg->l1_desc[arm_smmu_strtab_l1_idx(sid)];
>  
>  	if (desc->l2ptr)
>  		return 0;
>  
> -	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
> -	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
> -
> -	desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &l2ptr_dma,
> -					  GFP_KERNEL);
> +	desc->l2ptr = dmam_alloc_coherent(
> +		smmu->dev, STRTAB_NUM_L2_STES * sizeof(struct arm_smmu_ste),
> +		&l2ptr_dma, GFP_KERNEL);

Since this series is mainly about clean-up, please can you be consistent
with the indentation style that the driver currently uses for multi-line
function invocations? I applied the whole series, but I struggled looking
at the resulting code (of course, there's no "right" way, but the driver
is written with whatever I'm used to so I'd really like to keep the
consistency).

Will