| Message ID | 1439191116.22743.1.camel@ingics.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Mon, Aug 10, 2015 at 03:18:36PM +0800, Axel Lin wrote: > IS_ENABLED should only be used for CONFIG_* symbols. > So use #ifdef DEBUG instead. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Nack. While the abuse is certainly wrong you're removing compiler coverage over code which is worse. PLease find a way to remove the IS_ENABLED abuse without causing the code in question to lose compiler coverage. Thanks,
2015-08-10 15:32 GMT+08:00 Herbert Xu <herbert@gondor.apana.org.au>: > On Mon, Aug 10, 2015 at 03:18:36PM +0800, Axel Lin wrote: >> IS_ENABLED should only be used for CONFIG_* symbols. >> So use #ifdef DEBUG instead. >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> > > Nack. While the abuse is certainly wrong you're removing compiler > coverage over code which is worse. > > PLease find a way to remove the IS_ENABLED abuse without causing > the code in question to lose compiler coverage. I don't get it. sahara_decode_status/sahara_dump_descriptors/sahara_dump_links functions are mainly debug code. In original code, if IS_ENABLED(DEBUG) is false, these functions return immediately. DEBUG is defined or not is compile time determinated, so I don't understand that "removing compiler coverage over code which is worse". -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c index 397a500b..b2661a5 100644 --- a/drivers/crypto/sahara.c +++ b/drivers/crypto/sahara.c @@ -384,11 +384,9 @@ static char *sahara_state[4] = { "Idle", "Busy", "Error", "HW Fault" }; static void sahara_decode_status(struct sahara_dev *dev, unsigned int status) { +#ifdef DEBUG u8 state; - if (!IS_ENABLED(DEBUG)) - return; - state = SAHARA_STATUS_GET_STATE(status); dev_dbg(dev->device, "%s: Status Register = 0x%08x\n", @@ -432,15 +430,14 @@ static void sahara_decode_status(struct sahara_dev *dev, unsigned int status) sahara_read(dev, SAHARA_REG_CDAR)); dev_dbg(dev->device, "Initial DAR: 0x%08x\n\n", sahara_read(dev, SAHARA_REG_IDAR)); +#endif } static void sahara_dump_descriptors(struct sahara_dev *dev) { +#ifdef DEBUG int i; - if (!IS_ENABLED(DEBUG)) - return; - for (i = 0; i < SAHARA_MAX_HW_DESC; i++) { dev_dbg(dev->device, "Descriptor (%d) (0x%08x):\n", i, dev->hw_phys_desc[i]); @@ -453,15 +450,14 @@ static void sahara_dump_descriptors(struct sahara_dev *dev) dev->hw_desc[i]->next); } dev_dbg(dev->device, "\n"); +#endif } static void sahara_dump_links(struct sahara_dev *dev) { +#ifdef DEBUG int i; - if (!IS_ENABLED(DEBUG)) - return; - for (i = 0; i < SAHARA_MAX_HW_LINK; i++) { dev_dbg(dev->device, "Link (%d) (0x%08x):\n", i, dev->hw_phys_link[i]); @@ -471,6 +467,7 @@ static void sahara_dump_links(struct sahara_dev *dev) dev->hw_link[i]->next); } dev_dbg(dev->device, "\n"); +#endif } static int sahara_hw_descriptor_create(struct sahara_dev *dev)
IS_ENABLED should only be used for CONFIG_* symbols. So use #ifdef DEBUG instead. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- drivers/crypto/sahara.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)