Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial
On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote: > Hi, > > Kernel defines same polynomial for CRC-32 in few places. > This is unnecessary duplication of the same value. Also this might > be error-prone for future code - every driver will define the > polynomial again. > > This is an attempt to unify definition of polynomial. Few obvious > hard-coded locations are fixed with define. > > All series depend on each 1/6 and 2/6. > > This could be merged in two different merge windows (1st lib/crc and then > the rest) or taken through one tree. > > It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 > and Freescale's Ethernet driver were tested on HW. Rest got just different > builds. > > Best regards, > Krzysztof > > > > > Krzysztof Kozlowski (6): > lib/crc: Move polynomial definition to separate header > lib/crc: Use consistent naming for CRC-32 polynomials > crypto: stm32_crc32 - Use existing define with polynomial > net: ethernet: Use existing define with polynomial > staging: rtl: Use existing define with polynomial > lib: Use existing define with polynomial > > drivers/crypto/stm32/stm32_crc32.c | 11 --- > drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++-- > drivers/net/ethernet/apple/bmac.c| 8 ++-- > drivers/net/ethernet/broadcom/tg3.c | 3 ++- > drivers/net/ethernet/freescale/fec_main.c| 4 ++-- > drivers/net/ethernet/freescale/fs_enet/fec.h | 3 --- > drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++- > drivers/net/ethernet/micrel/ks8851_mll.c | 3 ++- > drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c| 4 ++-- > drivers/staging/rtl8712/rtl871x_security.c | 5 ++--- > drivers/staging/rtl8723bs/core/rtw_security.c| 5 ++--- > include/linux/crc32poly.h| 20 > lib/crc32.c | 11 ++- > lib/crc32defs.h | 14 -- > lib/decompress_bunzip2.c | 3 ++- > lib/gen_crc32table.c | 5 +++-- > lib/xz/xz_crc32.c| 3 ++- > 17 files changed, 55 insertions(+), 54 deletions(-) > create mode 100644 include/linux/crc32poly.h All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial
On 18 July 2018 at 02:12, Eric Biggers wrote: > Hi Krzysztof, > > On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote: >> Hi, >> >> Kernel defines same polynomial for CRC-32 in few places. >> This is unnecessary duplication of the same value. Also this might >> be error-prone for future code - every driver will define the >> polynomial again. >> >> This is an attempt to unify definition of polynomial. Few obvious >> hard-coded locations are fixed with define. >> >> All series depend on each 1/6 and 2/6. >> >> This could be merged in two different merge windows (1st lib/crc and then >> the rest) or taken through one tree. >> >> It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 >> and Freescale's Ethernet driver were tested on HW. Rest got just different >> builds. >> >> Best regards, >> Krzysztof >> >> Krzysztof Kozlowski (6): >> lib/crc: Move polynomial definition to separate header >> lib/crc: Use consistent naming for CRC-32 polynomials >> crypto: stm32_crc32 - Use existing define with polynomial >> net: ethernet: Use existing define with polynomial >> staging: rtl: Use existing define with polynomial >> lib: Use existing define with polynomial >> >> drivers/crypto/stm32/stm32_crc32.c | 11 --- >> drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++-- >> drivers/net/ethernet/apple/bmac.c| 8 ++-- >> drivers/net/ethernet/broadcom/tg3.c | 3 ++- >> drivers/net/ethernet/freescale/fec_main.c| 4 ++-- >> drivers/net/ethernet/freescale/fs_enet/fec.h | 3 --- >> drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++- >> drivers/net/ethernet/micrel/ks8851_mll.c | 3 ++- >> drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c| 4 ++-- >> drivers/staging/rtl8712/rtl871x_security.c | 5 ++--- >> drivers/staging/rtl8723bs/core/rtw_security.c| 5 ++--- >> include/linux/crc32poly.h| 20 >> lib/crc32.c | 11 ++- >> lib/crc32defs.h | 14 -- >> lib/decompress_bunzip2.c | 3 ++- >> lib/gen_crc32table.c | 5 +++-- >> lib/xz/xz_crc32.c| 3 ++- >> 17 files changed, 55 insertions(+), 54 deletions(-) >> create mode 100644 include/linux/crc32poly.h >> > > Did you check whether any of these users can be converted to use the CRC > implementations in lib/, so they wouldn't need the polynomial definition > themselves? I did not check but that's interesting point... The Ethernet drivers (xgbe, tg3, fec, ks8851, dwc-xlgmac) look like could be converted to generic implementation. The apple/bmac looks weird. The rtl WiFi drivers in long term can be converted to use generic lib80211 for encryption (see commit 0d4876f4e977 ("staging:r8188eu: Use lib80211 to encrypt (TKIP) tx frames")) but that is much bigger task. The remaining use the polynomials in different aspect: 1. XZ and BUNZIP use it to create CRC tables - probably generic gen_crc32table.c could be used, 2. stm32_crc32.c uses it to initialize HW CRC accelerator. I can work on Freescale FEC, xz and bunzip code because these I can test but I would prefer to do it as follow up. Best regards, Krzysztof
Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial
Hi Krzysztof, On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote: > Hi, > > Kernel defines same polynomial for CRC-32 in few places. > This is unnecessary duplication of the same value. Also this might > be error-prone for future code - every driver will define the > polynomial again. > > This is an attempt to unify definition of polynomial. Few obvious > hard-coded locations are fixed with define. > > All series depend on each 1/6 and 2/6. > > This could be merged in two different merge windows (1st lib/crc and then > the rest) or taken through one tree. > > It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 > and Freescale's Ethernet driver were tested on HW. Rest got just different > builds. > > Best regards, > Krzysztof > > Krzysztof Kozlowski (6): > lib/crc: Move polynomial definition to separate header > lib/crc: Use consistent naming for CRC-32 polynomials > crypto: stm32_crc32 - Use existing define with polynomial > net: ethernet: Use existing define with polynomial > staging: rtl: Use existing define with polynomial > lib: Use existing define with polynomial > > drivers/crypto/stm32/stm32_crc32.c | 11 --- > drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++-- > drivers/net/ethernet/apple/bmac.c| 8 ++-- > drivers/net/ethernet/broadcom/tg3.c | 3 ++- > drivers/net/ethernet/freescale/fec_main.c| 4 ++-- > drivers/net/ethernet/freescale/fs_enet/fec.h | 3 --- > drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++- > drivers/net/ethernet/micrel/ks8851_mll.c | 3 ++- > drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c| 4 ++-- > drivers/staging/rtl8712/rtl871x_security.c | 5 ++--- > drivers/staging/rtl8723bs/core/rtw_security.c| 5 ++--- > include/linux/crc32poly.h| 20 > lib/crc32.c | 11 ++- > lib/crc32defs.h | 14 -- > lib/decompress_bunzip2.c | 3 ++- > lib/gen_crc32table.c | 5 +++-- > lib/xz/xz_crc32.c| 3 ++- > 17 files changed, 55 insertions(+), 54 deletions(-) > create mode 100644 include/linux/crc32poly.h > Did you check whether any of these users can be converted to use the CRC implementations in lib/, so they wouldn't need the polynomial definition themselves? - Eric
[PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial
Hi, Kernel defines same polynomial for CRC-32 in few places. This is unnecessary duplication of the same value. Also this might be error-prone for future code - every driver will define the polynomial again. This is an attempt to unify definition of polynomial. Few obvious hard-coded locations are fixed with define. All series depend on each 1/6 and 2/6. This could be merged in two different merge windows (1st lib/crc and then the rest) or taken through one tree. It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds. Best regards, Krzysztof Krzysztof Kozlowski (6): lib/crc: Move polynomial definition to separate header lib/crc: Use consistent naming for CRC-32 polynomials crypto: stm32_crc32 - Use existing define with polynomial net: ethernet: Use existing define with polynomial staging: rtl: Use existing define with polynomial lib: Use existing define with polynomial drivers/crypto/stm32/stm32_crc32.c | 11 --- drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++-- drivers/net/ethernet/apple/bmac.c| 8 ++-- drivers/net/ethernet/broadcom/tg3.c | 3 ++- drivers/net/ethernet/freescale/fec_main.c| 4 ++-- drivers/net/ethernet/freescale/fs_enet/fec.h | 3 --- drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++- drivers/net/ethernet/micrel/ks8851_mll.c | 3 ++- drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c| 4 ++-- drivers/staging/rtl8712/rtl871x_security.c | 5 ++--- drivers/staging/rtl8723bs/core/rtw_security.c| 5 ++--- include/linux/crc32poly.h| 20 lib/crc32.c | 11 ++- lib/crc32defs.h | 14 -- lib/decompress_bunzip2.c | 3 ++- lib/gen_crc32table.c | 5 +++-- lib/xz/xz_crc32.c| 3 ++- 17 files changed, 55 insertions(+), 54 deletions(-) create mode 100644 include/linux/crc32poly.h -- 2.14.1