2015-12-10 22:03:16

by Dan Carpenter

[permalink] [raw]
Subject: re: crypto: qat - add support for new devices to FW loader

Hello Pingchao Yang,

The patch b0272276d903: "crypto: qat - add support for new devices to
FW loader" from Dec 4, 2015, leads to the following static checker
warning:

drivers/crypto/qat/qat_common/qat_hal.c:421 qat_hal_check_ae_active()
warn: bitwise AND condition is false here

drivers/crypto/qat/qat_common/qat_hal.c
414 int qat_hal_check_ae_active(struct icp_qat_fw_loader_handle *handle,
415 unsigned int ae)
416 {
417 unsigned int enable = 0, active = 0;
418
419 qat_hal_rd_ae_csr(handle, ae, CTX_ENABLES, &enable);
420 qat_hal_rd_ae_csr(handle, ae, ACTIVE_CTX_STATUS, &active);
421 if ((enable & (0xff >> CE_ENABLE_BITPOS)) ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Never true.
CE_ENABLE_BITPOS is 8.
Perhaps the intention was to left shift like we do with ACS_ABO_BITPOS
on the next line?

422 (active & (1 << ACS_ABO_BITPOS)))
423 return 1;
424 else
425 return 0;
426 }

regards,
dan carpenter


2015-12-11 02:00:33

by Yang Pingchao

[permalink] [raw]
Subject: RE: crypto: qat - add support for new devices to FW loader

Hi Dan,

Yes, you are right. It should be left shift to check the AE ctx enable. Thanks for your defect, I will send a patch to fix this issue.

Best Regards,
Pingchao yang


-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, December 11, 2015 6:03 AM
To: Yang, Pingchao
Cc: qat-linux; [email protected]
Subject: re: crypto: qat - add support for new devices to FW loader

Hello Pingchao Yang,

The patch b0272276d903: "crypto: qat - add support for new devices to FW loader" from Dec 4, 2015, leads to the following static checker
warning:

drivers/crypto/qat/qat_common/qat_hal.c:421 qat_hal_check_ae_active()
warn: bitwise AND condition is false here

drivers/crypto/qat/qat_common/qat_hal.c
414 int qat_hal_check_ae_active(struct icp_qat_fw_loader_handle *handle,
415 unsigned int ae)
416 {
417 unsigned int enable = 0, active = 0;
418
419 qat_hal_rd_ae_csr(handle, ae, CTX_ENABLES, &enable);
420 qat_hal_rd_ae_csr(handle, ae, ACTIVE_CTX_STATUS, &active);
421 if ((enable & (0xff >> CE_ENABLE_BITPOS)) ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Never true.
CE_ENABLE_BITPOS is 8.
Perhaps the intention was to left shift like we do with ACS_ABO_BITPOS on the next line?

422 (active & (1 << ACS_ABO_BITPOS)))
423 return 1;
424 else
425 return 0;
426 }

regards,
dan carpenter