Re: [PATCH 2/2] efi: Capsule update support

[Sam Protsenko]

> When I originally wrote this patch in 2013 arm64 support didn't exist,
> and ia64 isn't going to be using capsule support. I can separate that
> out into a separate patch though, no problem.

For me it's just the matter of good VCS practices. In this case I call
this "patch atomicity" (one patch per feature). It's not about your
patch particularly, it's just policy. In the end it boils down to next
two things:
 1. Separating common code from platform code makes it easier to use
"git bisect" in case of regressions.
 2. This way if we want to revert patch, we can revert only stuff we
want, not touching another part (e.g. you want to revert platform
code, you can keep common code in place).

> Next time, could you please quote the part of the patch you're
> commenting on inline? That would have saved me searching through the
> original email.

Sure, my bad. I know it's general approach in mailing lists to review
patch, just forgot it.


On 13 October 2014 12:53, Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote:
> On Fri, 10 Oct, at 06:55:49PM, Sam Protsenko wrote:
>> Hi Matt,
>>
>> 1. Why x86 code isn't separated to another patch?
>
> When I originally wrote this patch in 2013 arm64 support didn't exist,
> and ia64 isn't going to be using capsule support. I can separate that
> out into a separate patch though, no problem.
>
>> 2. drivers/firmware/efi/reboot.c: efi_reboot():
>>     One shouldn't use "printk()" with no KERN_* stuff passed into it.
>> I'd recommend to use "pr_info()" macro or something like that.
>
> Oops, I missed that, good catch.
>
> Next time, could you please quote the part of the patch you're
> commenting on inline? That would have saved me searching through the
> original email.
>
> --
> Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/