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. > > Arnd Yes, we will fix caller. Caller api is not safe on 64bit. Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int, but it should be unsigned long.