Re: [Xen-devel] [PATCH v7 1/2] xen/arm: extend fdt_property_interrupts to support DomU

[Julien Grall <julien.grall@xxxxxxx>]

Hi Viktor,

On 07/08/2019 11:10, Viktor Mitin wrote:
> Extend fdt_property_interrupts to deal with other domain than the hwdom.
>
> The prototype of fdt_property_interrupts() has been modified
> to support both hwdom and domU in one function.
To be pedantic, the current prototype for fdt_property_interrupts() can already 
deal with either hwdom and DomU. What your patch does is passing kinfo, so you 
only use parameter rather than two. So how about:
"The domain and fdt can be found in the structure kinfo. Rather than adding a an 
extra argument for the domain, pass directly kinfo. This also requires to adapt 
fdt_property_interrupts() prototype."
> This is a preparatory for the next patch which consolidates
> make_timer_node and make_timer_domU_node".
In v3, I pointed out that it is a bit risky to write down the title of a patch 
that does not preceded it (or been committed). Imagine we can decide to rename 
it. Furthermore, "next patch" implies they are committed one after the other.
It is fairly common to apply part of the series if the rest needs some rework. 
So a better (and safer wording) is "A follow-up patch will need to create the 
interrupts for either Dom0 or DomU".
> Original goal is to consolidate make_timer functions.
With my suggestion above, this sentence can be dropped.

The rest of the patch looks good to me. I am happy to update the commit message 
while committing it.
Let me know your preference.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel