| Message ID | 79D75E042AE5B03F+20240229100449.1001261-1-wangyuli@uniontech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2] LoongArch/crypto: Clean up useless assignment operations | expand |
WangYuli <wangyuli@uniontech.com> wrote: > The LoongArch CRC32 hw acceleration is based on > arch/mips/crypto/crc32-mips.c. While the MIPS code supports both > MIPS32 and MIPS64, LA32 lacks the CRC instruction. As a result, > "line len -= sizeof(u32)" is unnecessary. > > Removing it can make context code style more unified and improve > code readability. > > Suggested-by: Guan Wentao <guanwentao@uniontech.com> > Signed-off-by: WangYuli <wangyuli@uniontech.com> > --- > arch/loongarch/crypto/crc32-loongarch.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/loongarch/crypto/crc32-loongarch.c b/arch/loongarch/crypto/crc32-loongarch.c > index a49e507af38c..3eebea3a7b47 100644 > --- a/arch/loongarch/crypto/crc32-loongarch.c > +++ b/arch/loongarch/crypto/crc32-loongarch.c > @@ -44,7 +44,6 @@ static u32 crc32_loongarch_hw(u32 crc_, const u8 *p, unsigned int len) > > CRC32(crc, value, w); > p += sizeof(u32); > - len -= sizeof(u32); > } This makes no sense whatsoever. Please review this patch carefully before you resubmit. Thanks,
On Fri, Mar 08, 2024 at 06:19:02PM +0800, Herbert Xu wrote: > > > diff --git a/arch/loongarch/crypto/crc32-loongarch.c b/arch/loongarch/crypto/crc32-loongarch.c > > index a49e507af38c..3eebea3a7b47 100644 > > --- a/arch/loongarch/crypto/crc32-loongarch.c > > +++ b/arch/loongarch/crypto/crc32-loongarch.c > > @@ -44,7 +44,6 @@ static u32 crc32_loongarch_hw(u32 crc_, const u8 *p, unsigned int len) > > > > CRC32(crc, value, w); > > p += sizeof(u32); > > - len -= sizeof(u32); > > } > > This makes no sense whatsoever. Please review this patch carefully > before you resubmit. Nevermind, I see what's going on now. Your reference to the lack of a CRC instruction utterly confused me. The fact that len -= is unnecessary has nothing to do with whether there is a CRC instruction. Please modify your patch description so that it is not needlessly confusing. You should simply state that len -= is unnecessary because you only test whether the relevant bit is set in len for the tail case. Thanks,
diff --git a/arch/loongarch/crypto/crc32-loongarch.c b/arch/loongarch/crypto/crc32-loongarch.c index a49e507af38c..3eebea3a7b47 100644 --- a/arch/loongarch/crypto/crc32-loongarch.c +++ b/arch/loongarch/crypto/crc32-loongarch.c @@ -44,7 +44,6 @@ static u32 crc32_loongarch_hw(u32 crc_, const u8 *p, unsigned int len) CRC32(crc, value, w); p += sizeof(u32); - len -= sizeof(u32); } if (len & sizeof(u16)) { @@ -80,7 +79,6 @@ static u32 crc32c_loongarch_hw(u32 crc_, const u8 *p, unsigned int len) CRC32C(crc, value, w); p += sizeof(u32); - len -= sizeof(u32); } if (len & sizeof(u16)) {
The LoongArch CRC32 hw acceleration is based on arch/mips/crypto/crc32-mips.c. While the MIPS code supports both MIPS32 and MIPS64, LA32 lacks the CRC instruction. As a result, "line len -= sizeof(u32)" is unnecessary. Removing it can make context code style more unified and improve code readability. Suggested-by: Guan Wentao <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com> --- arch/loongarch/crypto/crc32-loongarch.c | 2 -- 1 file changed, 2 deletions(-)