[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 6/7] drivers/passthrough/arm: Refactor code for arm smmu drivers


On 02/09/2018 11:02 AM, Roger Pau Monné wrote:
On Fri, Feb 09, 2018 at 10:51:01AM +0000, Julien Grall wrote:

On 02/09/2018 10:43 AM, Roger Pau Monné wrote:
+    unsigned int type;
+#define resource_size(res) ((res)->size)
+#define platform_device device
+/* Stub out DMA domain related functions */
+#define iommu_get_dma_cookie(dom) 0
+#define iommu_put_dma_cookie(dom)
+#define VA_BITS                0 /* Only used for configuring stage-1 input 
size */
+#define MODULE_DEVICE_TABLE(type, name)
+#define module_param_named(name, value, type, perm)
+#define MODULE_PARM_DESC(_parm, desc)
+#define dma_set_mask_and_coherent(d, b)        0
+#define of_dma_is_coherent(n)  0
+static void __iomem *devm_ioremap_resource(struct device *dev,
+                                          struct resource *res)

Aligment, please use spaces.

Also, is __iomem needed here at all?

On Arm, we tend to add keep __iomem on pointer dealing with MMIO.

I understand that you keep it when directly importing code from Linux,
but this is Xen code, so unless this is done merely for consistency it
seems quite pointless (__iomem is defined to nothing AFAICT).

We do it quite consistenly in Xen Arm code :). Have a look at the return of ioremap_* or read*/write* parameters.

While it might be defined to nothing today, I'd like to keep it because it could easily be defined to check the address space used.


+#endif /* __ARM_SMMU_H__ */
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
index f43485fe6e..f0a61521fb 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -49,28 +49,7 @@
   #include <asm/io.h>
   #include <asm/platform.h>
-/* Alias to Xen device tree helpers */
-#define device_node dt_device_node
-#define of_phandle_args dt_phandle_args
-#define of_device_id dt_device_match
-#define of_match_node dt_match_node
-#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, 
-#define of_property_read_bool dt_property_read_bool
-#define of_parse_phandle_with_args dt_parse_phandle_with_args
-/* Xen: Helpers to get device MMIO and IRQs */
-struct resource {
-       u64 addr;
-       u64 size;
-       unsigned int type;
-#define resource_size(res) ((res)->size)
-#define platform_device device

You introduce the above code in patch 5, and remove it in patch 6, is
this really needed?

Ie: why not simply introduce this code directly in this patch?

See https://lists.xen.org/archives/html/xen-devel/2018-01/msg02066.html

Hm, OK, I'm not sure I follow that.

AFAICT the above code is added in patch 5 so that the driver can be
hooked up into the build system. Could we just hold off hooking the
driver to the build system until patch 6, in order to avoid such
addition and removal of code?

What matters is to know what is common between SMMUv2 and SMMUv3 driver. So it can be pulled in a separate headers.

IHMO, the both yours and his way are valid. TBH, I would have done a third way and move that patch before #5. But at this stage, this is a matter of taste, hence way I didn't push to reshuffle the series.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.