Re: [PATCH 1/7] iommu/arm-smmu: Reorganize arm_smmu_domain_add_master()

From: Jason Gunthorpe <jgg@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	Heiko Stuebner <heiko@sntech.de>, Joerg Roedel <jroedel@suse.de>,
	Jerry Snitselaar <jsnitsel@redhat.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Niklas Schnelle <schnelle@linux.ibm.com>,
	Steven Price <steven.price@arm.com>
Subject: Re: [PATCH 1/7] iommu/arm-smmu: Reorganize arm_smmu_domain_add_master()
Date: Fri, 6 Oct 2023 09:30:01 -0300	[thread overview]
Message-ID: <20231006123001.GO682044@nvidia.com> (raw)
In-Reply-To: <a75f54d3-cd16-499d-b6d2-eed38153b0e2@arm.com>

On Fri, Oct 06, 2023 at 01:05:03PM +0100, Robin Murphy wrote:
> On 2023-10-05 19:28, Jason Gunthorpe wrote:
> > Make arm_smmu_domain_add_master() not use the smmu_domain to detect the
> > s2cr configuration, instead pass it in as a parameter. It always returns
> > zero so make it return void.
> 
> It doesn't follow that a function named arm_smmu_domain_<operation>() should
> not operate on an arm_smmu_domain... I think this is the point to rename it
> to something like arm_smmu_master_install_s2crs() to reflect that what it's
> actually doing by now is a lot less than it did 10 years ago.

heh, yes, I did not notice

> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index d6d1a2a55cc069..7f33363719f4ac 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -1081,21 +1081,14 @@ static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg,
> >   	mutex_unlock(&smmu->stream_map_mutex);
> >   }
> > -static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
> > -				      struct arm_smmu_master_cfg *cfg,
> > -				      struct iommu_fwspec *fwspec)
> > +static void arm_smmu_domain_add_master(struct arm_smmu_device *smmu,
> 
> We already have the SMMU device in cfg->smmu, no need to pass it twice.

Sure, but the caller also already has it on its stack..

Thanks,
Jason