Re: [PATCH 3/4] xen/ppc: Implement early serial printk on pseries

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

On 6/22/23 11:43 AM, Julien Grall wrote:
> Hi,

Thanks for the response. I had one more question with regards to the
emacs footer.

>All files usually have an emacs magic block. E.g.:
>
>/*
> * Local variables:
> * mode: C
> * c-file-style: "BSD"
> * c-basic-offset: 4
> * indent-tabs-mode: nil
> * End:
> */

Is this absolutely necessary to include? The CODING_STYLE document seems
to imply that it's optional and a quick peek at other architectures'
code suggests that this is the case.

If it is required, should it also be included in assembly files, or only
.c/.h ones?

>>> This seems to be refactoring. It would be best if this is done in a
>>> separate patch as this is easier to review.
>>
>> Following Andrew's suggestion earlier, I've split this patch into two
>> with the first one only doing the bare minimum to get an infinite loop
>> going in C rather than assembly. That first patch still includes the
>> refactoring of this trampoline into a macro, but the overall patch size
>> is much smaller. Do you think that would be fine to review, or would you
>> like a third commit for only this trampoline's refactoring?
> 
> In general we tend to split code movement/refactoring from new code.
> This helps during reviews. Not sure how small will be your patch. If it
> is only a few dozen of lines, then it should be fine :).

Makes sense, thanks.

> Cheers,

Thanks,
Shawn