lkml.org 
[lkml]   [2011]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 2/2] iommu/exynos: Add iommu driver for Exynos Platforms
From
On Fri, Oct 14, 2011 at 11:17 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> On Thu, Oct 13, 2011 at 04:13:23AM -0400, KyongHo Cho wrote:
>> +struct exynos_iommu_domain {
>> +       struct device *dev;
>> +       unsigned long *pgtable;
>> +       spinlock_t lock;
>> +       spinlock_t pgtablelock;
>> +};
>
> Can you please add comments to document what these spinlocks protect?
>
'lock' protects race condition on the domain private data.
'pgtablelock' does it on the page table of the domain.
Since updating page table does not need to be synchronized with
domain private data, I defined those 2 locks seperately.

I will add this information in the source code as comments.

>> +static int exynos_iommu_attach_device(struct iommu_domain *domain,
>> +                                  struct device *dev)
>> +{
>> +       struct exynos_iommu_domain *priv = domain->priv;
>> +       int ret;
>> +
>> +       spin_lock(&priv->lock);
>> +
>> +       priv->dev = dev;
>> +
>> +       spin_unlock(&priv->lock);
>> +
>> +       ret = exynos_iommu_enable(domain);
>> +
>> +       return ret;
>> +}
>> +
>> +static void exynos_iommu_detach_device(struct iommu_domain *domain,
>> +                                   struct device *dev)
>> +{
>> +       struct exynos_iommu_domain *priv = domain->priv;
>> +
>> +       spin_lock(&priv->lock);
>> +
>> +       if (priv->dev == dev) {
>> +               priv->dev = NULL;
>> +
>> +               spin_unlock(&priv->lock);
>> +
>> +               exynos_iommu_disable(domain);
>> +       } else {
>> +               spin_unlock(&priv->lock);
>> +       }
>> +}
>
> That looks weird. As I read this code there is a 1-1 mapping between a
> device and a domain. This breaks semantics of the iommu-api where a
> domain can contain multiple devices.

I agree.
I will implement it soon.


Thank you

KyongHo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-10-15 03:59    [from the cache]
©2003-2011 Jasper Spaans