Re: [PATCH v2 06/18] loongarch/crc32: expose CRC32 functions through lib

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2024/11/3 21:57, Eric Biggers wrote:
On Sun, Nov 03, 2024 at 09:36:55PM +0800, WangYuli wrote:
Even though the narrower CRC instructions doesn't require GRLEN=64, they still *aren't* part of LA32 (LoongArch reference manual v1.10, Volume 1, Table 2-1).
Link: https://lore.kernel.org/all/0a7d0a9e-c56e-4ee2-a83b-00164a450abe@xxxxxxxxxx/

Therefore, we could not directly add ARCH_HAS_CRC32 to config LOONGARCH.

There's still a runtime CPU feature check of cpu_has(CPU_FEATURE_CRC32).
See arch/loongarch/lib/crc32-loongarch.c.  So it's the same as before.
ARCH_HAS_CRC32 just means that the file will be compiled.

If you're trying to say that you think this file should be built only when
CONFIG_64BIT=y, then that would be an existing bug since the existing file
arch/loongarch/crypto/crc32-loongarch.c was built for both 32-bit and 64-bit.
But if you think this is a bug, I can fix this too.

- Eric


Actually, my originally mean is that directly declaring LoongArch ARCH_HAS_CRC32 without distinguishing between 32-bit and 64-bit might mislead those reading the code. And it's not rigorous. However, according to Huacai Chen's recent reply, there are many similar issues and they won't cause build errors for now. Link: https://lore.kernel.org/all/CAAhV-H5KaXBr-TdpDbJwcr_L0_mbSw=4J30uwQ2xn2YDs=Hg2Q@xxxxxxxxxxxxxx/
So, this change should be fine for now.

Reviewed-by: WangYuli <wangyuli@xxxxxxxxxxxxx>

Thanks,
--
WangYuli

Attachment: OpenPGP_0xC5DA1F3046F40BEE.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux