On Thu, Jul 07, 2016 at 10:31:11PM +0530, Arvind Yadav wrote:
-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying toAre they ? In the upstream kernel, they seem to be "u32".
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
So before assginment need a proper type casting.
-Yes, I have changed as per you suggestion.
-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'Not really sure I understand if/how this applies to the patch in question.
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.
I don't see an int passed to IS_ERR_VALUE(), I only see u32.
-Passing an 'unsigned short' or 'unsigned int'argument intoWhat does this have to do with this patch ?
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.
-Any user will get compilation warning for that do not pass anSure, but that doesn't mean that typecasting the parameter to unsigned long
unsigned long' argument.
does any good (other than hiding the real bug).
Your subject line still does not list the affected subsystem and/or driver.
Documentation/SubmittingPatches might give some hints about proper subject
lines, and looking at other patches applied to the same file(s) might help
as well.
Also, if you want someone to review your patches, it helps to Cc: that
someone.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@xxxxxxxxx>I don't see the point of this typecast.
---
drivers/soc/fsl/qe/ucc_fast.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..98eed25 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -267,8 +267,10 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
- qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
- if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+ (unsigned int)qe_muram_alloc(uf_info->utfs,
+ UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);If sizeof(u32) == sizeof(unsigned long), this patch does not have an effect.
+ if (IS_ERR_VALUE(
+ (unsigned long)uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
If sizeof(u32) < sizeof(unsigned long), it does not change anything, and the
resulting code is as wrong as it was before.
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -278,10 +280,11 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct ucc_fast_private ** ucc
/* Allocate memory for Rx Virtual Fifo */
uccf->ucc_fast_rx_virtual_fifo_base_offset =
- qe_muram_alloc(uf_info->urfs +
+ (unsigned int)qe_muram_alloc(uf_info->urfs +
UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
- if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+ if (IS_ERR_VALUE(
+ (unsigned long)uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;