Re: [PATCH 02/19] dma-iommu: cleanup dma-iommu.h
On Wed, Feb 06, 2019 at 03:08:26PM +, Robin Murphy wrote: > Other than dma-iommu.c itself, none of them *require* it - only arch/arm64 > selects it (the one from MTK_IOMMU is just bogus), and a lot of the drivers > also build for at least one other architecture (and/or arm64 with > !IOMMU_API). > > Either way, I have no vehement objection to the change, I just don't see > any positive value in it. I've moved the idef back down below the includes. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/19] dma-iommu: cleanup dma-iommu.h
On 01/02/2019 16:13, Christoph Hellwig wrote: On Fri, Feb 01, 2019 at 02:47:17PM +, Robin Murphy wrote: On 14/01/2019 09:41, Christoph Hellwig wrote: No need for a __KERNEL__ guard outside uapi, make sure we pull in the includes unconditionally so users can rely on it, and add a missing comment describing the #else cpp statement. Last but not least include instead of the asm version, which is frowned upon. I think the __KERNEL__ and asm/errno.h slip-ups are things I cargo-culted from the arch code as a fresh-faced noob yet to learn the finer details, so ack for those parts. The forward-declarations, though, were a deliberate effort to minimise header dependencies and compilation bloat for includers who absolutely wouldn't care, and specifically to try to avoid setting transitive include expectations since they always seem to end up breaking someone's config somewhere down the line. Admittedly this little backwater is hardly comparable to the likes of the sched.h business, but I'm still somewhat on the fence about that change :/ As far as I can tell almost all users of linux/dma-iommu.h require CONFIG_DMA_IOMMU to be enabled anyway.. Other than dma-iommu.c itself, none of them *require* it - only arch/arm64 selects it (the one from MTK_IOMMU is just bogus), and a lot of the drivers also build for at least one other architecture (and/or arm64 with !IOMMU_API). Either way, I have no vehement objection to the change, I just don't see any positive value in it. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/19] dma-iommu: cleanup dma-iommu.h
On Fri, Feb 01, 2019 at 02:47:17PM +, Robin Murphy wrote: > On 14/01/2019 09:41, Christoph Hellwig wrote: >> No need for a __KERNEL__ guard outside uapi, make sure we pull in the >> includes unconditionally so users can rely on it, and add a missing >> comment describing the #else cpp statement. Last but not least include >> instead of the asm version, which is frowned upon. > > I think the __KERNEL__ and asm/errno.h slip-ups are things I cargo-culted > from the arch code as a fresh-faced noob yet to learn the finer details, so > ack for those parts. The forward-declarations, though, were a deliberate > effort to minimise header dependencies and compilation bloat for includers > who absolutely wouldn't care, and specifically to try to avoid setting > transitive include expectations since they always seem to end up breaking > someone's config somewhere down the line. Admittedly this little backwater > is hardly comparable to the likes of the sched.h business, but I'm still > somewhat on the fence about that change :/ As far as I can tell almost all users of linux/dma-iommu.h require CONFIG_DMA_IOMMU to be enabled anyway.. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/19] dma-iommu: cleanup dma-iommu.h
On 14/01/2019 09:41, Christoph Hellwig wrote:
No need for a __KERNEL__ guard outside uapi, make sure we pull in the
includes unconditionally so users can rely on it, and add a missing
comment describing the #else cpp statement. Last but not least include
instead of the asm version, which is frowned upon.
I think the __KERNEL__ and asm/errno.h slip-ups are things I
cargo-culted from the arch code as a fresh-faced noob yet to learn the
finer details, so ack for those parts. The forward-declarations, though,
were a deliberate effort to minimise header dependencies and compilation
bloat for includers who absolutely wouldn't care, and specifically to
try to avoid setting transitive include expectations since they always
seem to end up breaking someone's config somewhere down the line.
Admittedly this little backwater is hardly comparable to the likes of
the sched.h business, but I'm still somewhat on the fence about that
change :/
Robin.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-iommu.h | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index e760dc5d1fa8..65aa888c2768 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -16,15 +16,13 @@
#ifndef __DMA_IOMMU_H
#define __DMA_IOMMU_H
-#ifdef __KERNEL__
-#include
-#include
-
-#ifdef CONFIG_IOMMU_DMA
+#include
#include
#include
#include
+#include
+#ifdef CONFIG_IOMMU_DMA
int iommu_dma_init(void);
/* Domain management interface for IOMMU drivers */
@@ -74,11 +72,7 @@ void iommu_dma_unmap_resource(struct device *dev, dma_addr_t
handle,
void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
-#else
-
-struct iommu_domain;
-struct msi_msg;
-struct device;
+#else /* CONFIG_IOMMU_DMA */
static inline int iommu_dma_init(void)
{
@@ -108,5 +102,4 @@ static inline void iommu_dma_get_resv_regions(struct device
*dev, struct list_he
}
#endif /* CONFIG_IOMMU_DMA */
-#endif /* __KERNEL__ */
#endif/* __DMA_IOMMU_H */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 02/19] dma-iommu: cleanup dma-iommu.h
No need for a __KERNEL__ guard outside uapi, make sure we pull in the
includes unconditionally so users can rely on it, and add a missing
comment describing the #else cpp statement. Last but not least include
instead of the asm version, which is frowned upon.
Signed-off-by: Christoph Hellwig
---
include/linux/dma-iommu.h | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index e760dc5d1fa8..65aa888c2768 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -16,15 +16,13 @@
#ifndef __DMA_IOMMU_H
#define __DMA_IOMMU_H
-#ifdef __KERNEL__
-#include
-#include
-
-#ifdef CONFIG_IOMMU_DMA
+#include
#include
#include
#include
+#include
+#ifdef CONFIG_IOMMU_DMA
int iommu_dma_init(void);
/* Domain management interface for IOMMU drivers */
@@ -74,11 +72,7 @@ void iommu_dma_unmap_resource(struct device *dev, dma_addr_t
handle,
void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
-#else
-
-struct iommu_domain;
-struct msi_msg;
-struct device;
+#else /* CONFIG_IOMMU_DMA */
static inline int iommu_dma_init(void)
{
@@ -108,5 +102,4 @@ static inline void iommu_dma_get_resv_regions(struct device
*dev, struct list_he
}
#endif /* CONFIG_IOMMU_DMA */
-#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
