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

[Eric Auger]

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

Eric
> 
> ?
> 
> Thanks,
> Jason
>