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
