Re: [PATCH v2 4/8] iommu/arm-smmu-v3: Prepare for SSID support - Jean-Philippe Brucker

From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, eric.auger@redhat.com,
	robin.murphy@arm.com, joro@8bytes.org, sudeep.holla@arm.com,
	rjw@rjwysocki.net, linux-acpi@vger.kernel.org,
	iommu@lists.linux-foundation.org, robh+dt@kernel.org,
	guohanjun@huawei.com, zhangfei.gao@linaro.org, will@kernel.org,
	linux-arm-kernel@lists.infradead.org, lenb@kernel.org
Subject: Re: [PATCH v2 4/8] iommu/arm-smmu-v3: Prepare for SSID support
Date: Fri, 22 Nov 2019 16:28:55 +0100	[thread overview]
Message-ID: <20191122152855.GA810215@lophozonia> (raw)
In-Reply-To: <20191111143811.000006cc@huawei.com>

On Mon, Nov 11, 2019 at 02:38:11PM +0000, Jonathan Cameron wrote:
> Hmm. There are several different refactors in here alongside a few new
> bits.  Would be nice to break it up more to make life even easier for
> reviewers.   It's not 'so' complex that it's really a problem though
> so could leave it as is if you really want to.

Sure, I'll see if I can split it more in next version.

> > +	table->ptr = dmam_alloc_coherent(smmu->dev, size, &table->ptr_dma,
> > +					 GFP_KERNEL | __GFP_ZERO);
> 
> We dropped dma_zalloc_coherent because we now zero in dma_alloc_coherent
> anyway.  Hence I'm fairly sure that __GFP_ZERO should have no effect.
> 
> https://lore.kernel.org/patchwork/patch/1031536/
> 
> Am I missing some special corner case here?

Here I just copied the GFP flags already in use. But removing all
__GFP_ZERO from the driver would make a good cleanup patch.

> > -	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
> > -		arm_smmu_write_ctx_desc(smmu, &smmu_domain->s1_cfg);
> > -
> 
> Whilst it seems fine, perhaps a note on the 'why' of moving this into
> finalise_s1 would be good in the patch description.

Ok. Since it's only to simplify the handling of allocation failure in a
subsequent patch, I think I'll move that part over there.

Thanks,
Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel