Hi Suman,
On Thursday 13 March 2014 17:16:07 Suman Anna wrote:
> On 03/07/2014 06:46 PM, Laurent Pinchart wrote:
> > The flush_iotlb_page() function prints a debug message when no
> > corresponding page was found in the TLB. That condition is incorrectly
> > checked and always resolves to true, given that the for_each_iotlb_cr()
> > loop is never interrupted and always reaches obj->nr_tlb_entries.
>
> Nice catch.
>
> > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > ---
> >
> > drivers/iommu/omap-iommu.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> > index bb605c9..cb1e1de 100644
> > --- a/drivers/iommu/omap-iommu.c
> > +++ b/drivers/iommu/omap-iommu.c
> > @@ -376,6 +376,7 @@ static void flush_iotlb_page(struct omap_iommu *obj,
> > u32 da)
> > {
> > int i;
> > struct cr_regs cr;
> > + bool found = false;
> >
> > pm_runtime_get_sync(obj->dev);
> >
> > @@ -394,11 +395,12 @@ static void flush_iotlb_page(struct omap_iommu *obj,
> > u32 da)
> > __func__, start, da, bytes);
> > iotlb_load_cr(obj, &cr);
> > iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
> > + found = true;
>
> The patch is fine as it is, but I think the loop can be breaked when an
> entry is found for simplification. I do not expect that we will have
> multiple entries with the same da in the TLBs (it is a multi-hit fault
> scenario) to continue looping through all entries. The only means for the
> multi-hit scenario to happen is user error with programming TLBs directly.
OK. I wasn't sure whether we could have two TLB entries for the same VA. I'll
update the patch accordingly.
> This function call seems to be added in general to take care of any
> prefetching. I don't have the complete history on the PREFETCH_IOTLB on why
> it was added, it doesn't seem to be enabled but this should ideally be left
> to the h/w unless you are locking a TLB entry. DSP/Bridge is the only one
> that uses locked TLB entries at the moment, but it is not using the OMAP
> IOMMU driver yet.
>
> > }
> > }
> > pm_runtime_put_sync(obj->dev);
> >
> > - if (i == obj->nr_tlb_entries)
> > + if (!found)
> > dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
> > }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
