| Message ID | 1461599894-1969-9-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On 2016-04-25 17:58, Sricharan R wrote: > Now that the device's iommu ops are configured at probe time, > the device has to be added to the iommu late. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- > drivers/of/device.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/of/device.c b/drivers/of/device.c > index 57a5f2d..722115c 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -6,6 +6,7 @@ > #include <linux/of_iommu.h> > #include <linux/dma-mapping.h> > #include <linux/init.h> > +#include <linux/iommu.h> > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/slab.h> > @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) > dev_dbg(dev, "device is%sbehind an iommu\n", > iommu ? " " : " not "); > > + if (iommu) > + iommu_bus_add_dev(dev); > + This causes build break when IOMMU subsystem is not enabled: drivers/of/device.c: In function 'of_dma_configure_ops': drivers/of/device.c:159:3: error: implicit declaration of function 'iommu_bus_add_dev' [-Werror=implicit-function-declaration] iommu_bus_add_dev(dev); ^ > arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent); > > return 0; Best regards
Hi Marek, >On 2016-04-25 17:58, Sricharan R wrote: >> Now that the device's iommu ops are configured at probe time, >> the device has to be added to the iommu late. >> >> Signed-off-by: Sricharan R <sricharan@codeaurora.org> >> --- >> drivers/of/device.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/of/device.c b/drivers/of/device.c >> index 57a5f2d..722115c 100644 >> --- a/drivers/of/device.c >> +++ b/drivers/of/device.c >> @@ -6,6 +6,7 @@ >> #include <linux/of_iommu.h> >> #include <linux/dma-mapping.h> >> #include <linux/init.h> >> +#include <linux/iommu.h> >> #include <linux/module.h> >> #include <linux/mod_devicetable.h> >> #include <linux/slab.h> >> @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) >> dev_dbg(dev, "device is%sbehind an iommu\n", >> iommu ? " " : " not "); >> >> + if (iommu) >> + iommu_bus_add_dev(dev); >> + > >This causes build break when IOMMU subsystem is not enabled: > >drivers/of/device.c: In function 'of_dma_configure_ops': >drivers/of/device.c:159:3: error: implicit declaration of function >'iommu_bus_add_dev' [-Werror=implicit-function-declaration] > iommu_bus_add_dev(dev); > ^ Ah ok. Thanks for pointing this out. I will repost, with this fixed. I also realised that PATCH 9, might cause a issue in NON-DT builds and needs a correction. But otherwise, i am waiting for suggestions/feedback on how to proceed with this series. Regards, Sricharan
On 25/04/16 16:58, Sricharan R wrote: > Now that the device's iommu ops are configured at probe time, > the device has to be added to the iommu late. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- > drivers/of/device.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/of/device.c b/drivers/of/device.c > index 57a5f2d..722115c 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -6,6 +6,7 @@ > #include <linux/of_iommu.h> > #include <linux/dma-mapping.h> > #include <linux/init.h> > +#include <linux/iommu.h> > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/slab.h> > @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) > dev_dbg(dev, "device is%sbehind an iommu\n", > iommu ? " " : " not "); > > + if (iommu) > + iommu_bus_add_dev(dev); This (in conjunction with the previous patch) seems unnecessarily convoluted - if of_iommu_configure() has found some iommu_ops for a device, why not just call .add_device() directly there and then? There are already systems that could warrant having two different IOMMU drivers active simultaneously (but thankfully don't _need_ to), so trying to escape from per-bus IOMMU ops makes more sense than entrenching the horrible notion of "the" IOMMU on "the" platform bus any further. Robin. > + > arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent); > > return 0; >
Hi Robin, >On 25/04/16 16:58, Sricharan R wrote: >> Now that the device's iommu ops are configured at probe time, >> the device has to be added to the iommu late. >> >> Signed-off-by: Sricharan R <sricharan@codeaurora.org> >> --- >> drivers/of/device.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/of/device.c b/drivers/of/device.c >> index 57a5f2d..722115c 100644 >> --- a/drivers/of/device.c >> +++ b/drivers/of/device.c >> @@ -6,6 +6,7 @@ >> #include <linux/of_iommu.h> >> #include <linux/dma-mapping.h> >> #include <linux/init.h> >> +#include <linux/iommu.h> >> #include <linux/module.h> >> #include <linux/mod_devicetable.h> >> #include <linux/slab.h> >> @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) >> dev_dbg(dev, "device is%sbehind an iommu\n", >> iommu ? " " : " not "); >> >> + if (iommu) >> + iommu_bus_add_dev(dev); > >This (in conjunction with the previous patch) seems unnecessarily >convoluted - if of_iommu_configure() has found some iommu_ops for a >device, why not just call .add_device() directly there and then? There >are already systems that could warrant having two different IOMMU >drivers active simultaneously (but thankfully don't _need_ to), so >trying to escape from per-bus IOMMU ops makes more sense than >entrenching the horrible notion of "the" IOMMU on "the" platform bus any >further. > Ok, agree on this. This will remove the addition of that api in previous patch as well. Will change this. Regards, Sricharan
diff --git a/drivers/of/device.c b/drivers/of/device.c index 57a5f2d..722115c 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -6,6 +6,7 @@ #include <linux/of_iommu.h> #include <linux/dma-mapping.h> #include <linux/init.h> +#include <linux/iommu.h> #include <linux/module.h> #include <linux/mod_devicetable.h> #include <linux/slab.h> @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); + if (iommu) + iommu_bus_add_dev(dev); + arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent); return 0;
Now that the device's iommu ops are configured at probe time, the device has to be added to the iommu late. Signed-off-by: Sricharan R <sricharan@codeaurora.org> --- drivers/of/device.c | 4 ++++ 1 file changed, 4 insertions(+)