On Tue, Feb 15, 2011 at 4:48 PM, Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx> wrote:
> Hi David,
Hi Hiroshi,
>
> Sorry for a bit late reply....
You're just in time. :)
>
> From: David Cohen <dacohen@xxxxxxxxx>
> 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 <dacohen@xxxxxxxxx>
>> ---
>> Â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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
