Hi Stephen, > -----Original Message----- > From: Stephen Boyd [mailto:sboyd@xxxxxxxxxxxxxx] > Sent: Thursday, August 13, 2015 12:42 AM > To: Sricharan R > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; > robin.murphy@xxxxxxx; robdclark@xxxxxxxxx; joro@xxxxxxxxxx; > srinivas.kandagatla@xxxxxxxxxx; laurent.pinchart@xxxxxxxxxxxxxxxx; > Will.Deacon@xxxxxxx; stepanm@xxxxxxxxxxxxxx; treding@xxxxxxxxxx > Subject: Re: [PATCH 3/5] iommu/msm: Add support for generic master > bindings > > On 08/12/2015 07:47 AM, Sricharan R wrote: > > @@ -702,6 +703,44 @@ static void print_ctx_regs(void __iomem *base, > int ctx) > > GET_PRRR(base, ctx), GET_NMRR(base, ctx)); > > } > > > > +static void insert_iommu_master(struct device *dev, > > + struct msm_iommu_dev *iommu, > > + struct of_phandle_args *spec) > > +{ > > + struct msm_iommu_ctx_dev *master; > > + int sid; > > + > > + master = kzalloc(sizeof(*master), GFP_KERNEL); > > This is called with irqs disabled, but it's not GFP_ATOMIC. Please test with > DEBUG_ATOMIC_SLEEP=y. Ok. I will have to adjust the locking here. It should be granular only for the list manipulation. > > > + master->of_node = dev->of_node; > > + list_add(&master->list, &iommu->ctx_list); > > + > > + for (sid = 0; sid < spec->args_count; sid++) > > + master->mids[sid] = spec->args[sid]; > > + > > + master->num_mids = spec->args_count; } > > + > > +static int qcom_iommu_of_xlate(struct device *dev, > > + struct of_phandle_args *spec) { > > + struct msm_iommu_dev *iommu; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&msm_iommu_lock, flags); > > + list_for_each_entry(iommu, &qcom_iommu_devices, dev_node) { > > + if (iommu->dev->of_node == spec->np) > > + break; > > + } > > The braces are unnecessary here. Ok, will remove this. > > > + > > + if (!iommu || (iommu->dev->of_node != spec->np)) > > Please remove extraneous parentheses. > Ok. > > + return -ENODEV; > > + > > + insert_iommu_master(dev, iommu, spec); > > + spin_unlock_irqrestore(&msm_iommu_lock, flags); > > + > > + return 0; > > +} > > + > > irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id) > > { > > struct msm_iommu_dev *iommu = dev_id; @@ -737,7 +776,7 @@ > fail: > > return 0; > > } > > > > -static const struct iommu_ops msm_iommu_ops = { > > +static struct iommu_ops msm_iommu_ops = { > > Is there a reason why we can't make of_iommu_set_ops() take a const ops > pointer? > Hmm right. of_iommu_set_ops is the one that needs change. I will add a separate patch to correct that. Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
