On 08/02/2016 10:34 AM, arvind Yadav wrote: > > > On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote: >> On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote: >>> On 08/01/2016 02:02 AM, Arnd Bergmann wrote: >>>>> diff --git a/include/linux/err.h b/include/linux/err.h >>>>> index 1e35588..c2a2789 100644 >>>>> --- a/include/linux/err.h >>>>> +++ b/include/linux/err.h >>>>> @@ -18,7 +18,17 @@ >>>>> >>>>> #ifndef __ASSEMBLY__ >>>>> >>>>> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned >>>>> long)-MAX_ERRNO) >>>>> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x)) >>>>> + >>>>> +static inline int is_error_check(unsigned long error) >>>> Please leave the existing macro alone. I think you were looking for >>>> something specific to the return code of qe_muram_alloc() function, >>>> so please add a helper in that subsystem if you need it, not in >>>> the generic header files. >>> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long. The >>> problem is certain callers that store the return value in a u32. Why >>> not just fix those callers to store it in unsigned long (at least until >>> error checking is done)? >>> >> Yes, that would also address another problem with code like >> >> kfree((void *)ugeth->tx_bd_ring_offset[i]); >> >> which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value >> that also holds the return value of qe_muram_alloc.
Well, hopefully it doesn't hold a return of qe_muram_alloc() when it's being passed to kfree()... There's also the code that casts kmalloc()'s return to u32, etc. ucc_geth is not 64-bit clean in general. >> >> Arnd > Yes, we will fix caller. Caller api is not safe on 64bit. The API is fine (or at least, I haven't seen a valid issue pointed out yet). The problem is the ucc_geth driver. > Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int, > but it should be unsigned long. cpm_muram_addr takes unsigned long as a parameter, not that it matters since you can't pass errors into it and a muram offset should never exceed 32 bits. -Scott
