Re: [Xen-devel] [PATCH] xen/arm: extend fdt_property_interrupts

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Viktor Mitin writes:

> Hi Volodymyr,
>
> On Fri, Jul 26, 2019 at 2:57 PM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>>
>> Hello Vikor,
>>
>> Viktor Mitin writes:
>>
>> > xen/arm: extend fdt_property_interrupts to deal with other
>> > domain than the hwdom
>>
>> It is unclear what problem you are facing. Could you please elaborate
>> commit message?
>
> Original goal is to merge make_timer_node and make_timer_domU_node functions.
> For background discussion please see discussion in mail (also
> mentioned in the patch):
> [PATCH] xen/arm: merge make_timer_node and make_timer_domU_node
> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00914.html
This is mentioned below the tearline and newer will be included in the
git history. Here is the rules:

 Write a good description for each patch:

 - The first line the top of the patch should contain a short
   description of what the patch does, and hints as to what code it
   touches

 - The description should be useful for both the reviewers and people in
   the future trying to understand what the patch does. It can be as
   short as Code cleanup -- no functional changes, or as long as it
   takes to accurately describe the bug you are trying to solve or the
   new functionality you are introducing.

It is hard to say what you are trying to solve with this change.

I can't review functional changes in your patch, because I don't
understand what you are trying to achieve.


>> Also, looks like your commit message is formatted incorrectly.
>>
> Please see below.
I was talking about commit message. I'm seeing "xen/arm:" tag both in
the subject and in the e-mail body.

>> > Tested with aarch64 qemu dom0 and dom0less boot.
>> >
>> > Suggested-by: Julien Grall <julien.grall@arm.com>
>> > Signed-off-by: Viktor Mitin <viktor_mitin@epam.com>
>> >
>> > ---
>> > See discussion in mail:
>> > [PATCH] xen/arm: merge make_timer_node and make_timer_domU_node
>> >
>> >
>> >  xen/arch/arm/domain_build.c | 22 +++++++++++++---------
>> >  1 file changed, 13 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> > index 4c8404155a..5f68603392 100644
>> > --- a/xen/arch/arm/domain_build.c
>> > +++ b/xen/arch/arm/domain_build.c
>> > @@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t interrupt,
>> >   *  "interrupts": contains the list of interrupts
>> >   *  "interrupt-parent": link to the GIC
>> >   */
>> > -static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
>> > -                                          unsigned num_irq)
>> > +static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
>> > +                            gic_interrupt_t *intr, unsigned num_irq)
>> Wrong indentation there.
>
> Such cases are not defined in the Coding Style document explicitly.
> See my clang-format coding style mail.
Yes, this is one of that rules that actually is not documented, but is
widely enforced.

--
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel