On Tue, Dec 03, 2024 at 08:53:31AM -0700, Simon Glass wrote:
Hi Tom,
On Tue, 3 Dec 2024 at 07:04, Tom Rini trini@konsulko.com wrote:
On Tue, Dec 03, 2024 at 06:46:11AM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 2 Dec 2024 at 17:37, Tom Rini trini@konsulko.com wrote:
On Mon, Dec 02, 2024 at 05:24:35PM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 2 Dec 2024 at 13:16, Tom Rini trini@konsulko.com wrote:
On Sun, Dec 01, 2024 at 08:24:22AM -0700, Simon Glass wrote: > Simplify a few expressions in this function. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > (no changes since v1) > > lib/efi_loader/efi_memory.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index f1154f73e05..3b1c7528e92 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -206,11 +206,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > (carve_desc->num_pages << EFI_PAGE_SHIFT); > > /* check whether we're overlapping */ > - if ((carve_end <= map_start) || (carve_start >= map_end)) > + if (carve_end <= map_start || carve_start >= map_end) > return EFI_CARVE_NO_OVERLAP; > > /* We're overlapping with non-RAM, warn the caller if desired */ > - if (overlap_conventional && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) > + if (overlap_conventional && map_desc->type != EFI_CONVENTIONAL_MEMORY) > return EFI_CARVE_OVERLAPS_NONRAM; > > /* Sanitize carve_start and carve_end to lie within our bounds */
As I believe was mentioned in a previous iteration, please drop this as they aren't excessive generates a compiler warning, merely for clarification and should be kept.
I did this patch because checkpatch complained and I am changing these lines.
And checkpatch is not the authority, it's guidelines.
Agreed.
I believe the review comments are "no, these should stay". Please drop this patch.
Just so I can figure out what to do here, are you saying:
- merge this patch in with the one that produces a checkpatch warning
(i.e. remove brackets so resolve warning), or
- drop this patch and ignore the checkpatch warning in the result
Yes, please just drop this patch, I only look at ERROR from checkpatch when merging things and then it's a matter of what the error is.
I don't really mind about this, obviously. But as I suspect this series is not going to be applied to your tree anyway, I'll await events.
Yes, if you're not going to be addressing most of the comments you get this is going to linger in, well, whatever it is you plan to be doing exactly.
I am keen to address comments, so long as they are about the code, rather than 'we don't want this patch'.
We don't want this patch because the parenthesis do make reviewing the code and order of operations clearer. This was the previous feedback.