On Tue, Feb 15, 2011 at 4:48 PM, Hiroshi DOYU <hiroshi.d...@nokia.com> wrote: > Hi David,
Hi Hiroshi, > > Sorry for a bit late reply.... You're just in time. :) > > From: David Cohen <daco...@gmail.com> > Subject: [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault > handling > Date: Tue, 15 Feb 2011 16:36:31 +0200 > >> Add support to register a callback for IOMMU fault situations. Drivers using >> IOMMU module might want to be informed when such errors happen in order to >> debug it or react. >> >> Signed-off-by: David Cohen <daco...@gmail.com> >> --- >> arch/arm/mach-omap2/iommu2.c | 21 +++++++++++++-- >> arch/arm/plat-omap/include/plat/iommu.h | 15 ++++++++++- >> arch/arm/plat-omap/iommu.c | 41 >> ++++++++++++++++++++++++++++--- >> 3 files changed, 69 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c >> index 14ee686..504310d 100644 >> --- a/arch/arm/mach-omap2/iommu2.c >> +++ b/arch/arm/mach-omap2/iommu2.c >> @@ -143,10 +143,10 @@ static void omap2_iommu_set_twl(struct iommu *obj, >> bool on) >> __iommu_set_twl(obj, false); >> } >> >> -static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >> +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 >> *iommu_errs) >> { >> int i; >> - u32 stat, da; >> + u32 stat, da, errs; >> const char *err_msg[] = { >> "tlb miss", >> "translation fault", >> @@ -157,8 +157,10 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 >> *ra) >> >> stat = iommu_read_reg(obj, MMU_IRQSTATUS); >> stat &= MMU_IRQ_MASK; >> - if (!stat) >> + if (!stat) { >> + *iommu_errs = 0; >> return 0; >> + } >> >> da = iommu_read_reg(obj, MMU_FAULT_AD); >> *ra = da; >> @@ -171,6 +173,19 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 >> *ra) >> } >> printk("\n"); >> >> + errs = 0; >> + if (stat & MMU_IRQ_TLBMISS) >> + errs |= OMAP_IOMMU_ERR_TLB_MISS; >> + if (stat & MMU_IRQ_TRANSLATIONFAULT) >> + errs |= OMAP_IOMMU_ERR_TRANS_FAULT; >> + if (stat & MMU_IRQ_EMUMISS) >> + errs |= OMAP_IOMMU_ERR_EMU_MISS; >> + if (stat & MMU_IRQ_TABLEWALKFAULT) >> + errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; >> + if (stat & MMU_IRQ_MULTIHITFAULT) >> + errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; >> + *iommu_errs = errs; >> + >> iommu_write_reg(obj, stat, MMU_IRQSTATUS); >> >> return stat; >> diff --git a/arch/arm/plat-omap/include/plat/iommu.h >> b/arch/arm/plat-omap/include/plat/iommu.h >> index 19cbb5e..5a2475f 100644 >> --- a/arch/arm/plat-omap/include/plat/iommu.h >> +++ b/arch/arm/plat-omap/include/plat/iommu.h >> @@ -31,6 +31,7 @@ struct iommu { >> struct clk *clk; >> void __iomem *regbase; >> struct device *dev; >> + void *fault_cb_priv; >> >> unsigned int refcount; >> struct mutex iommu_lock; /* global for this whole object */ >> @@ -48,6 +49,7 @@ struct iommu { >> struct mutex mmap_lock; /* protect mmap */ >> >> int (*isr)(struct iommu *obj); >> + void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void >> *priv); > > What about making use of (*isr)() for fault call back as well? > > Basic concept is that, client can decide how to deal with iommu > fault. For example, for advanced case, client wants daynamic loading of > TLB(or PTE), or for ISP case, clients just want more appropriate fault > reporting. This (*isr)() could be used in such flexibility. In this case, it seems we can re-use it. > > 785 * Device IOMMU generic operations > 786 */ > 787 static irqreturn_t iommu_fault_handler(int irq, void *data) > 788 { > 789 u32 stat, da; > 790 u32 *iopgd, *iopte; > 791 int err = -EIO; > 792 struct iommu *obj = data; > 793 > 794 if (!obj->refcount) > 795 return IRQ_NONE; > 796 > 797 /* Dynamic loading TLB or PTE */ > 798 if (obj->isr) > 799 err = obj->isr(obj); > 800 > 801 if (!err) > 802 return IRQ_HANDLED; > 803 > 804 clk_enable(obj->clk); > 805 stat = iommu_report_fault(obj, &da); > 806 clk_disable(obj->clk); > 807 if (!stat) > 808 return IRQ_HANDLED; > 809 > 810 iommu_disable(obj); > > I guess that this modifying the above code could make (*isr)() > flexible to be used for any purpose for clients? For me, having both > following may be a bit residual. I agree. We need to add 'void *isr_priv' to iommu struct and the function to register isr. I'll send a v3 soon. Regards, David > >> int (*isr)(struct iommu *obj); >> + void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void >> *priv); > -- 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