[PATCH V2 4/5] iommu/msm: use generic ARMV7S short descriptor pagetable ops

[Sricharan]

Hi will,
> -----Original Message-----
> From: Will Deacon [mailto:will.deacon at arm.com]
> Sent: Tuesday, April 26, 2016 7:47 PM
> To: Sricharan R <sricharan at codeaurora.org>
> Cc: linux-arm-kernel at lists.infradead.org;
iommu at lists.linux-foundation.org;
> devicetree at vger.kernel.org; linux-arm-msm at vger.kernel.org;
> robin.murphy at arm.com; robdclark at gmail.com; joro at 8bytes.org;
> srinivas.kandagatla at linaro.org; laurent.pinchart at ideasonboard.com;
> stepanm at codeaurora.org; treding at nvidia.com
> Subject: Re: [PATCH V2 4/5] iommu/msm: use generic ARMV7S short
> descriptor pagetable ops
> 
> On Wed, Apr 06, 2016 at 07:59:34PM +0530, Sricharan R wrote:
> > This iommu uses the armv7 short descriptor format. So use the generic
> > ARMV7S pagetable ops instead of rewriting the same stuff in the
> > driver.
> >
> > Signed-off-by: Sricharan R <sricharan at codeaurora.org>
> > ---
> >  drivers/iommu/Kconfig     |   1 +
> >  drivers/iommu/msm_iommu.c | 378
> > ++++++++++------------------------------------
> >  2 files changed, 80 insertions(+), 299 deletions(-)
> 
> Nice diffstat!
> 
> > +static void __flush_iotlb(void *cookie)
> >  {
> > -	struct msm_priv *priv = to_msm_priv(domain);
> > +	struct msm_priv *priv = cookie;
> >  	struct msm_iommu_dev *iommu = NULL;
> >  	struct msm_iommu_ctx_dev *master;
> >  	int ret = 0;
> >
> > -#ifndef CONFIG_IOMMU_PGTABLES_L2
> > -	unsigned long *fl_table = priv->pgtable;
> > -	int i;
> > -
> > -	if (!list_empty(&priv->list_attached)) {
> > -		dmac_flush_range(fl_table, fl_table + SZ_16K);
> > -
> > -		for (i = 0; i < NUM_FL_PTE; i++)
> > -			if ((fl_table[i] & 0x03) == FL_TYPE_TABLE) {
> > -				void *sl_table = __va(fl_table[i] &
> > -
> 	FL_BASE_MASK);
> > -				dmac_flush_range(sl_table, sl_table +
> SZ_4K);
> > -			}
> > -	}
> > -#endif
> > -
> >  	list_for_each_entry(iommu, &priv->list_attached, dom_node) {
> >  		ret = __enable_clocks(iommu);
> >  		if (ret)
> > @@ -162,9 +144,26 @@ static int __flush_iotlb(struct iommu_domain
> *domain)
> >  		__disable_clocks(iommu);
> >  	}
> >  fail:
> > -	return ret;
> > +	return;
> > +}
> > +
> > +static void __flush_iotlb_range(unsigned long iova, size_t size,
> > +				size_t granule, bool leaf, void *cookie) {
> > +	__flush_iotlb(cookie);
> 
> Can you really not do better than this? __flush_iotlb does a TLBIALL
afaict,
> whereas you have the address range here and
> drivers/iommu/msm_iommu_hw-8xxx.h has entries for things like
> TLBIVA[A]...
> 
> You might even be able to make SET_CTX_REG use writel_relaxed and stick a
> DSB in the sync callback (assuming that's sufficient to complete the
> maintenance on your device).
> 
  Yes, this is a overkill. I should change this as you have mentioned.
Thanks.
      
 
Regards,
  Sricharan