On Thu, Jul 10, 2014 at 5:53 PM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> wrote:
> On 07/10, Rob Clark wrote:
>> So, in it's current form, this is superficially a copy of msm_iommu
>> plus DT conversion. But the pre-DT IOMMU driver had fairly different
>> structure.. ie. psuedo root device, with IOMMU devices hanging off
>> that, and context devices hanging off that. The context devices were
>> what the client device would attach (which was also somewhat non-
>> standard.. see msm_iommu_get_ctx()).
>>
>> I couldn't really think of some sane way to refactor this and add DT
>> while at the same time keeping compatibility with the old pre-DT msm
>> stuff. So I copied to a new driver.
>>
>> It was pointed out that nothing upstream actually *used* the msm_iommu
>> driver. So if no one objects to dropping pre-DT support, then I could
>> do some patch rejuggling + sed to make this replace the old driver
>> instead.
>
> +1
>
>> +
>> +// TODO any good reason for global lock vs per-iommu lock?
>> +DEFINE_SPINLOCK(qcom_iommu_lock);
>
> static?
oh, yes.. ofc
I do wonder if we might want to make locking a bit more fine grained
to reduce contention (but, otoh, the gpu driver isn't going to contend
with itself, and other drivers probably aren't taxing the iommu quite
so hard). But I guess it would be ok to leave that as a future
optimization.
>
>> +static LIST_HEAD(qcom_iommu_devices);
>> +
>> +/* Note that a single iommu_domain can, for devices sitting behind
>> + * more than one IOMMU (ie. one per AXI interface) will have more
>> + * than one iommu in the iommu_list. But all are programmed to
>> + * point at the same pagetables so from client device perspective
>> + * they act as a single IOMMU.
>> + */
>> +struct qcom_domain_priv {
>> + unsigned long *pgtable;
>> + struct list_head iommu_list; /* list of attached 'struct qcom_iommu' */
>> +};
>> +
>> +static int __enable_clocks(struct qcom_iommu *iommu)
>> +{
>> + int ret;
>> +
>> + ret = clk_prepare_enable(iommu->pclk);
>
> Looks like __enable_clocks() is called from within a spinlock
> sometimes. Please move the prepare call outside of any atomic
> sections and also enable CONFIG_PROVE_LOCKING and/or
> CONFIG_DEBUG_ATOMIC_SLEEP to find such problems.
good point. I haven't gotten around to running this against a debug
kernel. It is only at the 'just barely works now' stage, I just
wanted to send for early feedback on the approach. (Mainly to make
sure no one screams bloody murder about dropping pre-DT support from
old driver.)
BR,
-R
>> + if (ret)
>> + goto fail;
>> +
>> + if (iommu->clk) {
>> + ret = clk_prepare_enable(iommu->clk);
>> + if (ret)
>> + clk_disable_unprepare(iommu->pclk);
>> + }
>> +fail:
>> + return ret;
>> +}
>> +
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
