Commit 800cd6fb authored by Rasmus Villemoes's avatar Rasmus Villemoes Committed by Li Yang

soc: fsl: qe: change return type of cpm_muram_alloc() to s32

There are a number of problems with cpm_muram_alloc() and its
callers. Most callers assign the return value to some variable and
then use IS_ERR_VALUE to check for allocation failure. However, when
that variable is not sizeof(long), this leads to warnings - and it is
indeed broken to do e.g.

  u32 foo = cpm_muram_alloc();
  if (IS_ERR_VALUE(foo))

on a 64-bit platform, since the condition

  foo >= (unsigned long)-ENOMEM

is tautologically false. There are also callers that ignore the
possibility of error, and then there are those that check for error by
comparing the return value to 0...

One could fix that by changing all callers to store the return value
temporarily in an "unsigned long" and test that. However, use of
IS_ERR_VALUE() is error-prone and should be restricted to things which
are inherently long-sized (stuff in pt_regs etc.). Instead, let's aim
for changing to the standard kernel style

  int foo = cpm_muram_alloc();
  if (foo < 0)
    deal_with_it()
  some->where = foo;

Changing the return type from unsigned long to s32 (aka signed int)
doesn't change the value that gets stored into any of the callers'
variables except if the caller was storing the result in a u64 _and_
the allocation failed, so in itself this patch should be a no-op.

Another problem with cpm_muram_alloc() is that it can certainly
validly return 0 - and except if some cpm_muram_alloc_fixed() call
interferes, the very first cpm_muram_alloc() call will return just
that. But that shows that both ucc_slow_free() and ucc_fast_free() are
buggy, since they assume that a value of 0 means "that field was never
allocated". We'll later change cpm_muram_free() to accept (and ignore)
a negative offset, so callers can use a sentinel of -1 instead of 0
and just unconditionally call cpm_muram_free().
Reviewed-by: default avatarTimur Tabi <timur@kernel.org>
Signed-off-by: default avatarRasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: default avatarLi Yang <leoyang.li@nxp.com>
parent 09a39ec9
...@@ -32,7 +32,7 @@ static phys_addr_t muram_pbase; ...@@ -32,7 +32,7 @@ static phys_addr_t muram_pbase;
struct muram_block { struct muram_block {
struct list_head head; struct list_head head;
unsigned long start; s32 start;
int size; int size;
}; };
...@@ -110,13 +110,14 @@ int cpm_muram_init(void) ...@@ -110,13 +110,14 @@ int cpm_muram_init(void)
* @algo: algorithm for alloc. * @algo: algorithm for alloc.
* @data: data for genalloc's algorithm. * @data: data for genalloc's algorithm.
* *
* This function returns an offset into the muram area. * This function returns a non-negative offset into the muram area, or
* a negative errno on failure.
*/ */
static unsigned long cpm_muram_alloc_common(unsigned long size, static s32 cpm_muram_alloc_common(unsigned long size,
genpool_algo_t algo, void *data) genpool_algo_t algo, void *data)
{ {
struct muram_block *entry; struct muram_block *entry;
unsigned long start; s32 start;
if (!muram_pool && cpm_muram_init()) if (!muram_pool && cpm_muram_init())
goto out2; goto out2;
...@@ -137,7 +138,7 @@ static unsigned long cpm_muram_alloc_common(unsigned long size, ...@@ -137,7 +138,7 @@ static unsigned long cpm_muram_alloc_common(unsigned long size,
out1: out1:
gen_pool_free(muram_pool, start, size); gen_pool_free(muram_pool, start, size);
out2: out2:
return (unsigned long)-ENOMEM; return -ENOMEM;
} }
/* /*
...@@ -145,13 +146,14 @@ static unsigned long cpm_muram_alloc_common(unsigned long size, ...@@ -145,13 +146,14 @@ static unsigned long cpm_muram_alloc_common(unsigned long size,
* @size: number of bytes to allocate * @size: number of bytes to allocate
* @align: requested alignment, in bytes * @align: requested alignment, in bytes
* *
* This function returns an offset into the muram area. * This function returns a non-negative offset into the muram area, or
* a negative errno on failure.
* Use cpm_dpram_addr() to get the virtual address of the area. * Use cpm_dpram_addr() to get the virtual address of the area.
* Use cpm_muram_free() to free the allocation. * Use cpm_muram_free() to free the allocation.
*/ */
unsigned long cpm_muram_alloc(unsigned long size, unsigned long align) s32 cpm_muram_alloc(unsigned long size, unsigned long align)
{ {
unsigned long start; s32 start;
unsigned long flags; unsigned long flags;
struct genpool_data_align muram_pool_data; struct genpool_data_align muram_pool_data;
...@@ -168,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc); ...@@ -168,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc);
* cpm_muram_free - free a chunk of multi-user ram * cpm_muram_free - free a chunk of multi-user ram
* @offset: The beginning of the chunk as returned by cpm_muram_alloc(). * @offset: The beginning of the chunk as returned by cpm_muram_alloc().
*/ */
int cpm_muram_free(unsigned long offset) int cpm_muram_free(s32 offset)
{ {
unsigned long flags; unsigned long flags;
int size; int size;
...@@ -194,13 +196,14 @@ EXPORT_SYMBOL(cpm_muram_free); ...@@ -194,13 +196,14 @@ EXPORT_SYMBOL(cpm_muram_free);
* cpm_muram_alloc_fixed - reserve a specific region of multi-user ram * cpm_muram_alloc_fixed - reserve a specific region of multi-user ram
* @offset: offset of allocation start address * @offset: offset of allocation start address
* @size: number of bytes to allocate * @size: number of bytes to allocate
* This function returns an offset into the muram area * This function returns @offset if the area was available, a negative
* errno otherwise.
* Use cpm_dpram_addr() to get the virtual address of the area. * Use cpm_dpram_addr() to get the virtual address of the area.
* Use cpm_muram_free() to free the allocation. * Use cpm_muram_free() to free the allocation.
*/ */
unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size) s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size)
{ {
unsigned long start; s32 start;
unsigned long flags; unsigned long flags;
struct genpool_data_fixed muram_pool_data_fixed; struct genpool_data_fixed muram_pool_data_fixed;
......
...@@ -98,26 +98,26 @@ static inline void qe_reset(void) {} ...@@ -98,26 +98,26 @@ static inline void qe_reset(void) {}
int cpm_muram_init(void); int cpm_muram_init(void);
#if defined(CONFIG_CPM) || defined(CONFIG_QUICC_ENGINE) #if defined(CONFIG_CPM) || defined(CONFIG_QUICC_ENGINE)
unsigned long cpm_muram_alloc(unsigned long size, unsigned long align); s32 cpm_muram_alloc(unsigned long size, unsigned long align);
int cpm_muram_free(unsigned long offset); int cpm_muram_free(s32 offset);
unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size); s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
void __iomem *cpm_muram_addr(unsigned long offset); void __iomem *cpm_muram_addr(unsigned long offset);
unsigned long cpm_muram_offset(void __iomem *addr); unsigned long cpm_muram_offset(void __iomem *addr);
dma_addr_t cpm_muram_dma(void __iomem *addr); dma_addr_t cpm_muram_dma(void __iomem *addr);
#else #else
static inline unsigned long cpm_muram_alloc(unsigned long size, static inline s32 cpm_muram_alloc(unsigned long size,
unsigned long align) unsigned long align)
{ {
return -ENOSYS; return -ENOSYS;
} }
static inline int cpm_muram_free(unsigned long offset) static inline int cpm_muram_free(s32 offset)
{ {
return -ENOSYS; return -ENOSYS;
} }
static inline unsigned long cpm_muram_alloc_fixed(unsigned long offset, static inline s32 cpm_muram_alloc_fixed(unsigned long offset,
unsigned long size) unsigned long size)
{ {
return -ENOSYS; return -ENOSYS;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment