Hi Stephen,
> -----Original Message-----
> From: Stephen Boyd [mailto:[email protected]]
> Sent: Thursday, August 13, 2015 12:42 AM
> To: Sricharan R
> Cc: [email protected];
[email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> 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 linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
