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

Re: [PATCH v2 8/8] xen/arm: Add support for SMMUv3 driver



Hi Stefano,

On 02/12/2020 02:51, Stefano Stabellini wrote:
On Thu, 26 Nov 2020, Rahul Singh wrote:

+/* 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, 
out))
+#define of_property_read_bool dt_property_read_bool
+#define of_parse_phandle_with_args dt_parse_phandle_with_args

Given all the changes to the file by the previous patches we are
basically fully (or almost fully) adapting this code to Xen.

So at that point I wonder if we should just as well make these changes
(e.g. s/of_phandle_args/dt_phandle_args/g) to the code too.

I have already accepted the fact that keeping Linux code as-is is nearly impossible without much workaround :). The benefits tends to also limited as we noticed for the SMMU driver.

I would like to point out that this may make quite difficult (if not impossible) to revert the previous patches which remove support for some features (e.g. atomic, MSI, ATS).

If we are going to adapt the code to Xen (I'd like to keep Linux code style though), then I think we should consider to keep code that may be useful in the near future (at least MSI, ATS).


+#define FIELD_GET(_mask, _reg)          \
+    (typeof(_mask))(((_reg) & (_mask)) >> (__builtin_ffsll(_mask) - 1))
+
+#define WRITE_ONCE(x, val)                  \
+do {                                        \
+    *(volatile typeof(x) *)&(x) = (val);    \
+} while (0)

maybe we should define this in xen/include/xen/lib.h

I have attempted such discussion in the past and this resulted to more bikeshed than it is worth it. So I would suggest to re-implement WRITE_ONCE() as write_atomic() for now.

Cheers,

--
Julien Grall



 


Rackspace

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