[PATCH v2 01/19] iommu/arm-smmu-v3: Add a type for the STE

[Jason Gunthorpe]

On Mon, Nov 27, 2023 at 06:51:21PM +0100, Eric Auger wrote:
> Hi Jason,
> 
> On 11/27/23 18:42, Jason Gunthorpe wrote:
> > On Mon, Nov 27, 2023 at 05:03:36PM +0100, Eric Auger wrote:
> > 
> >>> -static __le64 *arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)
> >>> +static struct arm_smmu_ste *
> >>> +arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)
> >>>  {
> >>> -	__le64 *step;
> >>>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> >>>  
> >>>  	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
> >>> -		struct arm_smmu_strtab_l1_desc *l1_desc;
> >>>  		int idx;
> >>>  
> >>>  		/* Two-level walk */
> >>>  		idx = (sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS;
> >>> -		l1_desc = &cfg->l1_desc[idx];
> >>> -		idx = (sid & ((1 << STRTAB_SPLIT) - 1)) * STRTAB_STE_DWORDS;
> >>> -		step = &l1_desc->l2ptr[idx];
> >>> +		return &cfg->l1_desc[idx].l2ptr[sid & ((1 << STRTAB_SPLIT) - 1)];
> >> This looks less readable to me than it was before.
> > 
> > You would like the idx calculation outside?
> > 
> >   		idx1 = (sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS;
> > 		idx2 = sid & ((1 << STRTAB_SPLIT) - 1);
> > 		return &cfg->l1_desc[idx].l2ptr[idx2];
> Yes this looks more readable to me

done

Jason