Hello Stefano, > On 3 Dec 2020, at 6:47 pm, Stefano Stabellini <sstabell...@kernel.org> wrote: > > On Thu, 3 Dec 2020, Rahul Singh wrote: >>> On 3 Dec 2020, at 4:13 am, Stefano Stabellini <sstabell...@kernel.org> >>> wrote: >>> On Wed, 2 Dec 2020, Julien Grall wrote: >>>> 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). >>> >>> (I am fine with keeping the Linux code style.) >>> >>> We could try to keep the code as similar to Linux as possible. This >>> didn't work out in the past. >>> >>> Otherwise, we could fully adapt the driver to Xen. If we fully adapt the >>> driver to Xen (code style aside) it is better to be consistent and also >>> do substitutions like s/of_phandle_args/dt_phandle_args/g. Then the >>> policy becomes clear: the code comes from Linux but it is 100% adapted >>> to Xen. >>> >>> >>> Now the question about what to do about the MSI and ATS code is a good >>> one. We know that we are going to want that code at some point in the >>> next 2 years. Like you wrote, if we fully adapt the code to Xen and >>> remove MSI and ATS code, then it is going to be harder to add it back. >>> >>> So maybe keeping the MSI and ATS code for now, even if it cannot work, >>> would be better. I think this strategy works well if the MSI and ATS >>> code can be disabled easily, i.e. with a couple of lines of code in the >>> init function rather than #ifdef everywhere. It doesn't work well if we >>> have to add #ifdef everywhere. >>> >>> It looks like MSI could be disabled adding a couple of lines to >>> arm_smmu_setup_msis. >>> >>> Similarly ATS seems to be easy to disable by forcing ats_enabled to >>> false. >>> >>> So yes, this looks like a good way forward. Rahul, what do you think? >> >> >> I am ok to have the PCI ATS and MSI functionality in the code. >> As per the discussion next version of the patch will include below >> modification:Please let me know if there are any suggestion overall that >> should be added in next version. >> >> 1. Keep the PCI ATS and MSI functionality code. > > I'll repeat one point I wrote above that I think it is important: please > try to disable ATS and MSI without invasive changes, ideally just a > couple of lines to force-disable each feature
Yes I will disable the feature. > > >> 2. Make the code with XEN compatible ( remove linux compatibility functions) >> 3. Keep the Linux coding style for code imported from Linux. >> 4. Fix all comments. > > Sounds good. > > >> I have one query what will be coding style for new code to make driver work >> for XEN ? > > We try to keep the same code style for the entirety of a source file. In > this case, the whole driver should use Linux code style (both imported > code and new code). Ok. Regards, Rahul
