[PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
The driver doesn't need to do anything important in device add/remove
callbacks, because initialization will be done from device-tree specific
callbacks added later. IOMMU groups created by current code were never
used.
Signed-off-by: Marek Szyprowski
---
drivers/iommu/exynos-iommu.c | 28
1 file changed, 28 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index e62cb96..e40e699 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1055,32 +1055,6 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct
iommu_domain *domain,
return phys;
}
-static int exynos_iommu_add_device(struct device *dev)
-{
- struct iommu_group *group;
- int ret;
-
- group = iommu_group_get(dev);
-
- if (!group) {
- group = iommu_group_alloc();
- if (IS_ERR(group)) {
- dev_err(dev, "Failed to allocate IOMMU group\n");
- return PTR_ERR(group);
- }
- }
-
- ret = iommu_group_add_device(group, dev);
- iommu_group_put(group);
-
- return ret;
-}
-
-static void exynos_iommu_remove_device(struct device *dev)
-{
- iommu_group_remove_device(dev);
-}
-
static const struct iommu_ops exynos_iommu_ops = {
.domain_init = exynos_iommu_domain_init,
.domain_destroy = exynos_iommu_domain_destroy,
@@ -1090,8 +1064,6 @@ static const struct iommu_ops exynos_iommu_ops = {
.unmap = exynos_iommu_unmap,
.map_sg = default_iommu_map_sg,
.iova_to_phys = exynos_iommu_iova_to_phys,
- .add_device = exynos_iommu_add_device,
- .remove_device = exynos_iommu_remove_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
};
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
Hi Marek,
Thank you for the patch.
On Friday 23 January 2015 16:51:21 Marek Szyprowski wrote:
> The driver doesn't need to do anything important in device add/remove
> callbacks, because initialization will be done from device-tree specific
> callbacks added later. IOMMU groups created by current code were never
> used.
IOMMU groups still seem a bit unclear to me. Will Deacon has nicely explained
what they represent in
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310816.html.
The IOMMU core doesn't make groups
mandatory, but requires them in some code paths.
For example the coldplug device add function add_iommu_group() called for all
devices already registered when bus_set_iommu() is called will try to warn of
devices added multiple times with a WARN_ON(dev->iommu_group). Another example
is the iommu_bus_notifier() function which will call the remove_device()
operation only when dev->iommu_group isn't NULL.
I'm thus unsure whether groups should be made mandatory, or whether the IOMMU
core should be fixed to make them really optional (or, third option, whether
there's something I haven't understood properly).
> Signed-off-by: Marek Szyprowski
> ---
> drivers/iommu/exynos-iommu.c | 28
> 1 file changed, 28 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index e62cb96..e40e699 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1055,32 +1055,6 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct
> iommu_domain *domain, return phys;
> }
>
> -static int exynos_iommu_add_device(struct device *dev)
> -{
> - struct iommu_group *group;
> - int ret;
> -
> - group = iommu_group_get(dev);
> -
> - if (!group) {
> - group = iommu_group_alloc();
> - if (IS_ERR(group)) {
> - dev_err(dev, "Failed to allocate IOMMU group\n");
> - return PTR_ERR(group);
> - }
> - }
> -
> - ret = iommu_group_add_device(group, dev);
> - iommu_group_put(group);
> -
> - return ret;
> -}
> -
> -static void exynos_iommu_remove_device(struct device *dev)
> -{
> - iommu_group_remove_device(dev);
> -}
> -
> static const struct iommu_ops exynos_iommu_ops = {
> .domain_init = exynos_iommu_domain_init,
> .domain_destroy = exynos_iommu_domain_destroy,
> @@ -1090,8 +1064,6 @@ static const struct iommu_ops exynos_iommu_ops = {
> .unmap = exynos_iommu_unmap,
> .map_sg = default_iommu_map_sg,
> .iova_to_phys = exynos_iommu_iova_to_phys,
> - .add_device = exynos_iommu_add_device,
> - .remove_device = exynos_iommu_remove_device,
> .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
> };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
Hi Laurent, On Sun, Jan 25, 2015 at 05:38:22PM +0200, Laurent Pinchart wrote: > IOMMU groups still seem a bit unclear to me. Will Deacon has nicely explained > what they represent in > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310816.html. > The IOMMU core doesn't make groups > mandatory, but requires them in some code paths. > > For example the coldplug device add function add_iommu_group() called for all > devices already registered when bus_set_iommu() is called will try to warn of > devices added multiple times with a WARN_ON(dev->iommu_group). Another > example > is the iommu_bus_notifier() function which will call the remove_device() > operation only when dev->iommu_group isn't NULL. > > I'm thus unsure whether groups should be made mandatory, or whether the IOMMU > core should be fixed to make them really optional (or, third option, whether > there's something I haven't understood properly). My plan is to make IOMMU groups mandatory. I am currently preparing and RFC patch-set to introduce default-domains (which will be per group). So when all IOMMU drivers are converted to make use of default domains the iommu groups will be mandatory. Joerg -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
On Mon, Jan 26, 2015 at 11:00:59AM +, Joerg Roedel wrote: > On Sun, Jan 25, 2015 at 05:38:22PM +0200, Laurent Pinchart wrote: > > IOMMU groups still seem a bit unclear to me. Will Deacon has nicely > > explained > > what they represent in > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310816.html. > > The IOMMU core doesn't make groups > > mandatory, but requires them in some code paths. > > > > For example the coldplug device add function add_iommu_group() called for > > all > > devices already registered when bus_set_iommu() is called will try to warn > > of > > devices added multiple times with a WARN_ON(dev->iommu_group). Another > > example > > is the iommu_bus_notifier() function which will call the remove_device() > > operation only when dev->iommu_group isn't NULL. > > > > I'm thus unsure whether groups should be made mandatory, or whether the > > IOMMU > > core should be fixed to make them really optional (or, third option, > > whether > > there's something I haven't understood properly). > > My plan is to make IOMMU groups mandatory. I am currently preparing and > RFC patch-set to introduce default-domains (which will be per group). So > when all IOMMU drivers are converted to make use of default domains the > iommu groups will be mandatory. That makes sense to me, too. I think we should also consider extending the generic IOMMU device-tree binding to describe groups so that they can be instantiated by core code, without each driver having to work things out for itself. Will -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
Hello, On 2015-01-26 12:00, Joerg Roedel wrote: Hi Laurent, On Sun, Jan 25, 2015 at 05:38:22PM +0200, Laurent Pinchart wrote: IOMMU groups still seem a bit unclear to me. Will Deacon has nicely explained what they represent in http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310816.html. The IOMMU core doesn't make groups mandatory, but requires them in some code paths. For example the coldplug device add function add_iommu_group() called for all devices already registered when bus_set_iommu() is called will try to warn of devices added multiple times with a WARN_ON(dev->iommu_group). Another example is the iommu_bus_notifier() function which will call the remove_device() operation only when dev->iommu_group isn't NULL. I'm thus unsure whether groups should be made mandatory, or whether the IOMMU core should be fixed to make them really optional (or, third option, whether there's something I haven't understood properly). My plan is to make IOMMU groups mandatory. I am currently preparing and RFC patch-set to introduce default-domains (which will be per group). So when all IOMMU drivers are converted to make use of default domains the iommu groups will be mandatory. Thanks for the comment, I will implement all that will be needed for it to exynos iommu driver, but I would like to ask if you plan to merge my existing patches for exynos iommu driver to your tree? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
Hi Joerg, On Monday 26 January 2015 12:00:59 Joerg Roedel wrote: > Hi Laurent, > > On Sun, Jan 25, 2015 at 05:38:22PM +0200, Laurent Pinchart wrote: > > IOMMU groups still seem a bit unclear to me. Will Deacon has nicely > > explained what they represent in > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310816 > > .html. The IOMMU core doesn't make groups > > mandatory, but requires them in some code paths. > > > > For example the coldplug device add function add_iommu_group() called for > > all devices already registered when bus_set_iommu() is called will try to > > warn of devices added multiple times with a WARN_ON(dev->iommu_group). > > Another example is the iommu_bus_notifier() function which will call the > > remove_device() operation only when dev->iommu_group isn't NULL. > > > > I'm thus unsure whether groups should be made mandatory, or whether the > > IOMMU core should be fixed to make them really optional (or, third > > option, whether there's something I haven't understood properly). > > My plan is to make IOMMU groups mandatory. I am currently preparing and > RFC patch-set to introduce default-domains (which will be per group). So > when all IOMMU drivers are converted to make use of default domains the > iommu groups will be mandatory. Could the default domain policy be configured by the IOMMU driver ? The ipmmu- vmsa driver supports four domains only, and serves up to 32 masters, with one group per master. A default of one domain per group wouldn't be usable. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 11/18] iommu: exynos: remove useless device_add/remove callbacks
Hi Laurent, On Mon, Jan 26, 2015 at 03:03:57PM +0200, Laurent Pinchart wrote: > On Monday 26 January 2015 12:00:59 Joerg Roedel wrote: > > My plan is to make IOMMU groups mandatory. I am currently preparing and > > RFC patch-set to introduce default-domains (which will be per group). So > > when all IOMMU drivers are converted to make use of default domains the > > iommu groups will be mandatory. > > Could the default domain policy be configured by the IOMMU driver ? The ipmmu- > vmsa driver supports four domains only, and serves up to 32 masters, with one > group per master. A default of one domain per group wouldn't be usable. I guess this limitation is a property of the iommu hardware. I currently have no support for this in the patch-set, but I think it is easy (and the best way) to support a model where we have only one default domain for all groups. This is not in the patch set right now, but will be added at some point. Joerg -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
