Re: [PATCH v2 1/8] arm64: Import latest version of Cortex Strings' memcmp

From: Will Deacon <will@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
	catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/8] arm64: Import latest version of Cortex Strings' memcmp
Date: Tue, 1 Jun 2021 19:26:44 +0100	[thread overview]
Message-ID: <20210601182644.GA29456@willie-the-truck> (raw)
In-Reply-To: <20210527165214.GA39198@C02TD0UTHF1T.local>

On Thu, May 27, 2021 at 05:52:14PM +0100, Mark Rutland wrote:
> On Thu, May 27, 2021 at 04:34:41PM +0100, Robin Murphy wrote:
> > From: Sam Tebbs <sam.tebbs@arm.com>
> > 
> > Import the latest version of the former Cortex Strings - now
> > Arm Optimized Routines - memcmp function based on the upstream
> > code of string/aarch64/memcmp.S at commit e823e3a from
> > https://github.com/ARM-software/optimized-routines
> > 
> > Note that for simplicity Arm have chosen to contribute this code
> > to Linux under GPLv2 rather than the original MIT license.
> 
> Thanks for adding this! It would be nice if we could make this slightly
> more explicit, e.g.
> 
> | Arm hold the copyright on this code, and have chosen to contribute it
> | to linux under GPL v2 rather than the original MIT license.
> 
> ... but I don't think that requires a respin.
> 
> > 
> > Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
> > [ rm: update attribution and commit message ]
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >  arch/arm64/lib/memcmp.S | 330 ++++++++++++++--------------------------
> >  1 file changed, 111 insertions(+), 219 deletions(-)
> > 
> > diff --git a/arch/arm64/lib/memcmp.S b/arch/arm64/lib/memcmp.S
> > index c0671e793ea9..498f0d9941d9 100644
> > --- a/arch/arm64/lib/memcmp.S
> > +++ b/arch/arm64/lib/memcmp.S
> > @@ -1,247 +1,139 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> >  /*
> > - * Copyright (C) 2013 ARM Ltd.
> > - * Copyright (C) 2013 Linaro.
> > + * Copyright (c) 2013-2020, Arm Limited.
> 
> 2021 too, presumably. ;)
> 
> >   *
> > - * This code is based on glibc cortex strings work originally authored by Linaro
> > - * be found @
> > - *
> > - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
> > - * files/head:/src/aarch64/
> > + * Adapted from the original at:
> > + * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcmp.S
> >   */
> 
> Could we please note the commit ID in the comment, too? e.g. we can
> place that in the URL:
> 
> https://github.com/ARM-software/optimized-routines/blob/e823e3abf5f89ecb/string/aarch64/memcmp.S
> 
> I've checked that the below matches the code in the file referenced
> above, and AFAICT, the only change I see in the body of the function is
> dropping PTR_ARG() and SIZE_ARG(), which are not necessary as the kernel
> itself is not built with ILP32. The local definition of L() also seems
> fine to me.
> 
> On the assumption that the code was correct to begin with, this looks
> good to me. With the minor fixups above:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

I've applied this as-is with your Ack, but I haven't yet put it into
linux-next. Please can you (or Robin) send patches on top to address the
above?

Sorry for rushing a bit, but I'd like to get CKI chewing on this ASAP
and didn't want to wait for a respin if it's just cosmetic stuff pending.

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel