Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1577494 > unrolled thread
| Started by | Joerg Roedel <joro@8bytes.org> |
|---|---|
| First post | 2017-02-09 12:50 +0100 |
| Last post | 2017-02-10 16:40 +0100 |
| Articles | 4 on this page of 24 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 00/11 v3] Let IOMMU core know about individual IOMMUs Joerg Roedel <joro@8bytes.org> - 2017-02-09 12:50 +0100
[PATCH 02/11] iommu: Rename struct iommu_device Joerg Roedel <joro@8bytes.org> - 2017-02-09 12:50 +0100
[PATCH 08/11] iommu/msm: Make use of iommu_device_register interface Joerg Roedel <joro@8bytes.org> - 2017-02-09 12:50 +0100
Re: [PATCH 08/11] iommu/msm: Make use of iommu_device_register interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 15:40 +0100
Re: [PATCH 08/11] iommu/msm: Make use of iommu_device_register interface Joerg Roedel <joro@8bytes.org> - 2017-02-10 17:00 +0100
Re: [PATCH 08/11] iommu/msm: Make use of iommu_device_register interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 18:40 +0100
[PATCH 05/11] iommu: Make iommu_device_link/unlink take a struct iommu_device Joerg Roedel <joro@8bytes.org> - 2017-02-09 13:40 +0100
[PATCH 07/11] iommu/arm-smmu: Make use of the iommu_register interface Joerg Roedel <joro@8bytes.org> - 2017-02-09 13:40 +0100
Re: [PATCH 07/11] iommu/arm-smmu: Make use of the iommu_register interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 15:30 +0100
Re: [PATCH 07/11] iommu/arm-smmu: Make use of the iommu_register interface Joerg Roedel <joro@8bytes.org> - 2017-02-10 16:40 +0100
Re: [PATCH 07/11] iommu/arm-smmu: Make use of the iommu_register interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 18:10 +0100
[PATCH 04/11] iommu: Add sysfs bindings for struct iommu_device Joerg Roedel <joro@8bytes.org> - 2017-02-09 14:50 +0100
[PATCH 03/11] iommu: Introduce new 'struct iommu_device' Joerg Roedel <joro@8bytes.org> - 2017-02-09 14:50 +0100
Re: [PATCH 03/11] iommu: Introduce new 'struct iommu_device' kbuild test robot <lkp@intel.com> - 2017-02-09 21:50 +0100
[PATCH 09/11] iommu/mediatek: Make use of iommu_device_register interface Joerg Roedel <joro@8bytes.org> - 2017-02-09 14:50 +0100
[PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Joerg Roedel <joro@8bytes.org> - 2017-02-09 14:50 +0100
Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 15:20 +0100
Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Joerg Roedel <joro@8bytes.org> - 2017-02-10 16:30 +0100
Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 17:10 +0100
Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Joerg Roedel <jroedel@suse.de> - 2017-02-10 17:20 +0100
Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface Robin Murphy <robin.murphy@arm.com> - 2017-02-10 18:10 +0100
[PATCH 01/11] iommu: Rename iommu_get_instance() Joerg Roedel <joro@8bytes.org> - 2017-02-09 14:50 +0100
Re: [PATCH 01/11] iommu: Rename iommu_get_instance() Robin Murphy <robin.murphy@arm.com> - 2017-02-10 15:20 +0100
Re: [PATCH 01/11] iommu: Rename iommu_get_instance() Joerg Roedel <joro@8bytes.org> - 2017-02-10 16:40 +0100
Page 2 of 2 — ← Prev page 1 [2]
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Date | 2017-02-10 18:10 +0100 |
| Subject | Re: [PATCH 06/11] iommu: Add iommu_device_set_fwnode() interface |
| Message-ID | <t9mRQ-6is-19@gated-at.bofh.it> |
| In reply to | #1578584 |
On 10/02/17 16:11, Joerg Roedel wrote:
> On Fri, Feb 10, 2017 at 04:03:07PM +0000, Robin Murphy wrote:
>> Yeah, on reflection explicit initialisation is certainly easier to read
>> than a bunch of arguments handled implicitly by register(), but then
>> from that angle, even more clear would be to simply have the drivers
>> write the relevant struct members directly - I'd be quite happy with
>> that, and we then don't have to add another setter to iommu.h for every
>> new struct member (and risk it looking like Java code...)
>
> Yeah, that was my first approach. But there is the Intel VT-d anomaly,
> where a part of the driver can be built-in (dmar.c) with
> CONFIG_IOMMU_API=N. In this case 'struct iommu_device' is empty, and
> trying to access the members directly doesn't compile anymore.
>
> I have to look if this anomaly could be removed, then it is probably the
> best to set the struct members directly without wrapper functions.
Ah, I hadn't managed to spot that - I assume there probably is some
valid edge case for wanting x2APIC functionality without DMA remapping
which prevents us from just adding the dependency. Looking at the code,
though, that situation does seem to rely on the call never actually
executing at runtime - not only is it conditional on a static variable
which is only ever set by non-present code, it would fail the probe if
it were called - so I think it would be perfectly reasonable to just
address that particular problem as below (untested, but if it lets us
get rid of the dummy !IOMMU_API definitions of the registration
functions I'd say we've done the right thing).
Robin.
----->8-----
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 8ccbd7023194..161641caff79 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1077,6 +1077,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
raw_spin_lock_init(&iommu->register_lock);
+#ifdef CONFIG_IOMMU_API
if (intel_iommu_enabled) {
iommu->iommu_dev = iommu_device_create(NULL, iommu,
intel_iommu_groups,
@@ -1087,6 +1088,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
goto err_unmap;
}
}
+#endif
drhd->iommu = iommu;
[toc] | [prev] | [next] | [standalone]
| From | Joerg Roedel <joro@8bytes.org> |
|---|---|
| Date | 2017-02-09 14:50 +0100 |
| Subject | [PATCH 01/11] iommu: Rename iommu_get_instance() |
| Message-ID | <t8XgK-6Nm-27@gated-at.bofh.it> |
| In reply to | #1577494 |
From: Joerg Roedel <jroedel@suse.de>
Rename the function to iommu_ops_from_fwnode(), because that
is what the function actually does. The new name is much
more descriptive about what the function does.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/acpi/arm64/iort.c | 2 +-
drivers/iommu/iommu.c | 2 +-
include/linux/iommu.h | 4 ++--
include/linux/of_iommu.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index e0d2e6e..3752521 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -536,7 +536,7 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
if (!iort_fwnode)
return NULL;
- ops = iommu_get_instance(iort_fwnode);
+ ops = iommu_ops_from_fwnode(iort_fwnode);
if (!ops)
return NULL;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index dbe7f65..0ee05bb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1653,7 +1653,7 @@ void iommu_register_instance(struct fwnode_handle *fwnode,
spin_unlock(&iommu_instance_lock);
}
-const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
+const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
{
struct iommu_instance *instance;
const struct iommu_ops *ops = NULL;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0ff5111..085e1f0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -354,7 +354,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
void iommu_register_instance(struct fwnode_handle *fwnode,
const struct iommu_ops *ops);
-const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode);
+const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode);
#else /* CONFIG_IOMMU_API */
@@ -590,7 +590,7 @@ static inline void iommu_register_instance(struct fwnode_handle *fwnode,
}
static inline
-const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
+const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
{
return NULL;
}
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 6a7fc50..66fcbc9 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -39,7 +39,7 @@ static inline void of_iommu_set_ops(struct device_node *np,
static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
{
- return iommu_get_instance(&np->fwnode);
+ return iommu_ops_from_fwnode(&np->fwnode);
}
extern struct of_device_id __iommu_of_table;
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Date | 2017-02-10 15:20 +0100 |
| Subject | Re: [PATCH 01/11] iommu: Rename iommu_get_instance() |
| Message-ID | <t9kdj-4sU-7@gated-at.bofh.it> |
| In reply to | #1577607 |
Hi Joerg,
I'm really liking this series! Superficially it doesn't seem to break
anything on my Juno, but I'll give it a more thorough workout soon.
Just a few comments from skimming through...
On 09/02/17 11:32, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Rename the function to iommu_ops_from_fwnode(), because that
> is what the function actually does. The new name is much
> more descriptive about what the function does.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/acpi/arm64/iort.c | 2 +-
> drivers/iommu/iommu.c | 2 +-
> include/linux/iommu.h | 4 ++--
> include/linux/of_iommu.h | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index e0d2e6e..3752521 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -536,7 +536,7 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
> if (!iort_fwnode)
> return NULL;
>
> - ops = iommu_get_instance(iort_fwnode);
> + ops = iommu_ops_from_fwnode(iort_fwnode);
> if (!ops)
> return NULL;
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index dbe7f65..0ee05bb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1653,7 +1653,7 @@ void iommu_register_instance(struct fwnode_handle *fwnode,
> spin_unlock(&iommu_instance_lock);
> }
>
> -const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
> +const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
> {
> struct iommu_instance *instance;
> const struct iommu_ops *ops = NULL;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0ff5111..085e1f0 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -354,7 +354,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
> void iommu_register_instance(struct fwnode_handle *fwnode,
> const struct iommu_ops *ops);
> -const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode);
> +const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode);
>
> #else /* CONFIG_IOMMU_API */
>
> @@ -590,7 +590,7 @@ static inline void iommu_register_instance(struct fwnode_handle *fwnode,
> }
>
> static inline
> -const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
> +const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
> {
> return NULL;
> }
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 6a7fc50..66fcbc9 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -39,7 +39,7 @@ static inline void of_iommu_set_ops(struct device_node *np,
>
> static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> {
> - return iommu_get_instance(&np->fwnode);
> + return iommu_ops_from_fwnode(&np->fwnode);
> }
Note that you've already got Lorenzo's patch queued to remove these of_
wrappers.
Robin.
>
> extern struct of_device_id __iommu_of_table;
>
[toc] | [prev] | [next] | [standalone]
| From | Joerg Roedel <joro@8bytes.org> |
|---|---|
| Date | 2017-02-10 16:40 +0100 |
| Subject | Re: [PATCH 01/11] iommu: Rename iommu_get_instance() |
| Message-ID | <t9lsJ-5dQ-7@gated-at.bofh.it> |
| In reply to | #1578488 |
Hi Robin,
On Fri, Feb 10, 2017 at 02:12:54PM +0000, Robin Murphy wrote:
> I'm really liking this series! Superficially it doesn't seem to break
> anything on my Juno, but I'll give it a more thorough workout soon.
Great, thanks for testing! Any problems in this series should show up on
boot anyway.
> > static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > {
> > - return iommu_get_instance(&np->fwnode);
> > + return iommu_ops_from_fwnode(&np->fwnode);
> > }
>
> Note that you've already got Lorenzo's patch queued to remove these of_
> wrappers.
Yes, that was among the conflicts I resolved when merging this with the
rest of the iommu-tree.
Joerg
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web