On 28. Nov 2019, at 16:00, linuxppc-dev-request@lists.ozlabs.org wrote:
Send Linuxppc-dev mailing list submissions to
linuxppc-dev@lists.ozlabs.org
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.ozlabs.org/listinfo/linuxppc-dev
or, via email, send a message with subject or body 'help' to
linuxppc-dev-request@lists.ozlabs.org
You can reach the person managing the list at
linuxppc-dev-owner@lists.ozlabs.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Linuxppc-dev digest..."
Today's Topics:
1. [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return
void (Rasmus Villemoes)
2. [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a
negative offset (Rasmus Villemoes)
3. [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of
cpm_muram_init() (Rasmus Villemoes)
4. [PATCH v6 39/49] soc: fsl: qe: refactor
cpm_muram_alloc_common to prevent BUG on error path (Rasmus Villemoes)
5. [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in
ucc_slow.c (Rasmus Villemoes)
----------------------------------------------------------------------
Message: 1
Date: Thu, 28 Nov 2019 15:55:41 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <redacted>,
Christophe Leroy [off-list ref]
Cc: <redacted>,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood [off-list ref], Timur Tabi [off-list ref], Rasmus
Villemoes [off-list ref]
Send Linuxppc-dev mailing list submissions to
linuxppc-dev@lists.ozlabs.org
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.ozlabs.org/listinfo/linuxppc-dev
or, via email, send a message with subject or body 'help' to
linuxppc-dev-request@lists.ozlabs.org
You can reach the person managing the list at
linuxppc-dev-owner@lists.ozlabs.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Linuxppc-dev digest..."
Today's Topics:
1. [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return
void (Rasmus Villemoes)
2. [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a
negative offset (Rasmus Villemoes)
3. [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of
cpm_muram_init() (Rasmus Villemoes)
4. [PATCH v6 39/49] soc: fsl: qe: refactor
cpm_muram_alloc_common to prevent BUG on error path (Rasmus Villemoes)
5. [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in
ucc_slow.c (Rasmus Villemoes)
----------------------------------------------------------------------
Message: 1
Date: Thu, 28 Nov 2019 15:55:41 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <redacted>,
Christophe Leroy [off-list ref]
Cc: <redacted>,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood [off-list ref], Timur Tabi [off-list ref], Rasmus
Villemoes [off-list ref]
Subject: [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return
void
Message-ID: [off-list ref]
Nobody uses the return value from cpm_muram_free, and functions that
free resources usually return void. One could imagine a use for a "how
much have I allocated" a la ksize(), but knowing how much one had
access to after the fact is useless.
Reviewed-by: Timur Tabi <timur@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/soc/fsl/qe/qe_common.c | 3 +--
include/soc/fsl/qe/qe.h | 5 ++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 84c90105e588..962835488f66 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -170,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc);
* cpm_muram_free - free a chunk of multi-user ram
* @offset: The beginning of the chunk as returned by cpm_muram_alloc().
*/
-int cpm_muram_free(s32 offset)
+void cpm_muram_free(s32 offset)
{
unsigned long flags;
int size;@@ -188,7 +188,6 @@ int cpm_muram_free(s32 offset)
}
gen_pool_free(muram_pool, offset + GENPOOL_OFFSET, size);
spin_unlock_irqrestore(&cpm_muram_lock, flags);
- return size;
}
EXPORT_SYMBOL(cpm_muram_free);
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index f589ae3f1216..e282ac01ec08 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -99,7 +99,7 @@ int cpm_muram_init(void);
#if defined(CONFIG_CPM) || defined(CONFIG_QUICC_ENGINE)
s32 cpm_muram_alloc(unsigned long size, unsigned long align);
-int cpm_muram_free(s32 offset);
+void cpm_muram_free(s32 offset);
s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
void __iomem *cpm_muram_addr(unsigned long offset);
unsigned long cpm_muram_offset(void __iomem *addr);
@@ -111,9 +111,8 @@ static inline s32 cpm_muram_alloc(unsigned long size,
return -ENOSYS;
}
-static inline int cpm_muram_free(s32 offset)
+static inline void cpm_muram_free(s32 offset)
{
- return -ENOSYS;
}
static inline s32 cpm_muram_alloc_fixed(unsigned long offset,
--
2.23.0
------------------------------
Message: 2
Date: Thu, 28 Nov 2019 15:55:42 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <redacted>,
Christophe Leroy [off-list ref]
Cc: <redacted>,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood [off-list ref], Timur Tabi [off-list ref], Rasmus
Villemoes [off-list ref]
Subject: [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a
negative offset
Message-ID: [off-list ref]
This allows one to simplify callers since they can store a negative
value as a sentinel to indicate "this was never allocated" (or store
the -ENOMEM from an allocation failure) and then call cpm_muram_free()
unconditionally.
Reviewed-by: Timur Tabi <timur@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/soc/fsl/qe/qe_common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 962835488f66..48c77bb92846 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -176,6 +176,9 @@ void cpm_muram_free(s32 offset)
int size;
struct muram_block *tmp;
+ if (offset < 0)
+ return;
+
size = 0;
spin_lock_irqsave(&cpm_muram_lock, flags);
list_for_each_entry(tmp, &muram_block_list, head) {--
2.23.0
------------------------------
Message: 3
Date: Thu, 28 Nov 2019 15:55:43 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <leoyang.li@nxp.com>,
Christophe Leroy <christophe.leroy@c-s.fr>
Cc: linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood <oss@buserror.net>, Timur Tabi <timur@kernel.org>, Rasmus
Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of
cpm_muram_init()
Message-ID: <20191128145554.1297-39-linux@rasmusvillemoes.dk>
cpm_muram_alloc_common() tries to support a kind of lazy
initialization - if the muram_pool has not been created yet, it calls
cpm_muram_init(). Now, cpm_muram_alloc_common() is always called under
spin_lock_irqsave(&cpm_muram_lock, flags);
and cpm_muram_init() does gen_pool_create() (which implies a
GFP_KERNEL allocation) and ioremap(), not to mention the fun that
ensues from cpm_muram_init() doing
spin_lock_init(&cpm_muram_lock);
In other words, this has never worked, so nobody can have been relying
on it.
cpm_muram_init() is called from a subsys_initcall (either from
cpm_init() in arch/powerpc/sysdev/cpm_common.c or, via qe_reset(),
from qe_init() in drivers/soc/fsl/qe/qe.c).
Reviewed-by: Timur Tabi <timur@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/soc/fsl/qe/qe_common.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 48c77bb92846..dcb267567d76 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -119,9 +119,6 @@ static s32 cpm_muram_alloc_common(unsigned long size,
struct muram_block *entry;
s32 start;
- if (!muram_pool && cpm_muram_init())
- goto out2;
-
start = gen_pool_alloc_algo(muram_pool, size, algo, data);
if (!start)
goto out2;
--
2.23.0
------------------------------
Message: 4
Date: Thu, 28 Nov 2019 15:55:44 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <leoyang.li@nxp.com>,
Christophe Leroy <christophe.leroy@c-s.fr>
Cc: linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood <oss@buserror.net>, Timur Tabi <timur@kernel.org>, Rasmus
Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH v6 39/49] soc: fsl: qe: refactor
cpm_muram_alloc_common to prevent BUG on error path
Message-ID: <20191128145554.1297-40-linux@rasmusvillemoes.dk>
If the kmalloc() fails, we try to undo the gen_pool allocation we've
just done. Unfortunately, start has already been modified to subtract
the GENPOOL_OFFSET bias, so we're freeing something that very likely
doesn't exist in the gen_pool, meaning we hit the
kernel BUG at lib/genalloc.c:399!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
[<803fd0e8>] (gen_pool_free) from [<80426bc8>] (cpm_muram_alloc_common+0xb0/0xc8)
[<80426bc8>] (cpm_muram_alloc_common) from [<80426c28>] (cpm_muram_alloc+0x48/0x80)
[<80426c28>] (cpm_muram_alloc) from [<80428214>] (ucc_slow_init+0x110/0x4f0)
[<80428214>] (ucc_slow_init) from [<8044a718>] (qe_uart_request_port+0x3c/0x1d8)
(this was tested by just injecting a random failure by adding
"|| (get_random_int()&7) == 0" to the "if (!entry)" condition).
Refactor the code so we do the kmalloc() first, meaning that's the
thing that needs undoing in case gen_pool_alloc_algo() then
fails. This allows a later cleanup to move the locking from the
callers into the _common function, keeping the kmalloc() out of the
critical region and then, hopefully (if all the muram_alloc callers
allow) change it to a GFP_KERNEL allocation.
Reviewed-by: Timur Tabi <timur@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/soc/fsl/qe/qe_common.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index dcb267567d76..a81a1a79f1ca 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -119,23 +119,21 @@ static s32 cpm_muram_alloc_common(unsigned long size,
struct muram_block *entry;
s32 start;
+ entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!entry)
+ return -ENOMEM;
start = gen_pool_alloc_algo(muram_pool, size, algo, data);
- if (!start)
- goto out2;
+ if (!start) {
+ kfree(entry);
+ return -ENOMEM;
+ }
start = start - GENPOOL_OFFSET;
memset_io(cpm_muram_addr(start), 0, size);
- entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
- if (!entry)
- goto out1;
entry->start = start;
entry->size = size;
list_add(&entry->head, &muram_block_list);
return start;
-out1:
- gen_pool_free(muram_pool, start, size);
-out2:
- return -ENOMEM;
}
/*
--
2.23.0
------------------------------
Message: 5
Date: Thu, 28 Nov 2019 15:55:45 +0100
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Qiang Zhao <qiang.zhao@nxp.com>, Li Yang <leoyang.li@nxp.com>,
Christophe Leroy <christophe.leroy@c-s.fr>
Cc: linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
Scott Wood <oss@buserror.net>, Timur Tabi <timur@kernel.org>, Rasmus
Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in
ucc_slow.c
Message-ID: <20191128145554.1297-41-linux@rasmusvillemoes.dk>
When trying to build this for a 64-bit platform, one gets warnings
from using IS_ERR_VALUE on something which is not sizeof(long).
Instead, change the various *_offset fields to store a signed integer,
and simply check for a negative return from qe_muram_alloc(). Since
qe_muram_free() now accepts and ignores a negative argument, we only
need to make sure these fields are initialized with -1, and we can
just unconditionally call qe_muram_free() in ucc_slow_free().
Note that the error case for us_pram_offset failed to set that field
to 0 (which, as noted earlier, is anyway a bogus sentinel value).
Reviewed-by: Timur Tabi <timur@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/soc/fsl/qe/ucc_slow.c | 22 +++++++++-------------
include/soc/fsl/qe/ucc_slow.h | 6 +++---
2 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c
index 9b55fd0f50c6..274d34449846 100644
--- a/drivers/soc/fsl/qe/ucc_slow.c
+++ b/drivers/soc/fsl/qe/ucc_slow.c
@@ -154,6 +154,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc
__func__);
return -ENOMEM;
}
+ uccs->rx_base_offset = -1;
+ uccs->tx_base_offset = -1;
+ uccs->us_pram_offset = -1;
/* Fill slow UCC structure */
uccs->us_info = us_info;
@@ -179,7 +182,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc
/* Get PRAM base */
uccs->us_pram_offset =
qe_muram_alloc(UCC_SLOW_PRAM_SIZE, ALIGNMENT_OF_UCC_SLOW_PRAM);
- if (IS_ERR_VALUE(uccs->us_pram_offset)) {
+ if (uccs->us_pram_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for PRAM", __func__);
ucc_slow_free(uccs);
return -ENOMEM;
@@ -206,10 +209,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc
uccs->rx_base_offset =
qe_muram_alloc(us_info->rx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
- if (IS_ERR_VALUE(uccs->rx_base_offset)) {
+ if (uccs->rx_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate %u RX BDs\n", __func__,
us_info->rx_bd_ring_len);
- uccs->rx_base_offset = 0;
ucc_slow_free(uccs);
return -ENOMEM;
}
@@ -217,9 +219,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc
uccs->tx_base_offset =
qe_muram_alloc(us_info->tx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
- if (IS_ERR_VALUE(uccs->tx_base_offset)) {
+ if (uccs->tx_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate TX BDs", __func__);
- uccs->tx_base_offset = 0;
ucc_slow_free(uccs);
return -ENOMEM;
}
@@ -352,14 +353,9 @@ void ucc_slow_free(struct ucc_slow_private * uccs)
if (!uccs)
return;
- if (uccs->rx_base_offset)
- qe_muram_free(uccs->rx_base_offset);
-
- if (uccs->tx_base_offset)
- qe_muram_free(uccs->tx_base_offset);
-
- if (uccs->us_pram)
- qe_muram_free(uccs->us_pram_offset);
+ qe_muram_free(uccs->rx_base_offset);
+ qe_muram_free(uccs->tx_base_offset);
+ qe_muram_free(uccs->us_pram_offset);
if (uccs->us_regs)
iounmap(uccs->us_regs);
diff --git a/include/soc/fsl/qe/ucc_slow.h b/include/soc/fsl/qe/ucc_slow.h
index 8696fdea2ae9..d187a6be83bc 100644
--- a/include/soc/fsl/qe/ucc_slow.h
+++ b/include/soc/fsl/qe/ucc_slow.h
@@ -185,7 +185,7 @@ struct ucc_slow_private {
struct ucc_slow_info *us_info;
struct ucc_slow __iomem *us_regs; /* Ptr to memory map of UCC regs */
struct ucc_slow_pram *us_pram; /* a pointer to the parameter RAM */
- u32 us_pram_offset;
+ s32 us_pram_offset;
int enabled_tx; /* Whether channel is enabled for Tx (ENT) */
int enabled_rx; /* Whether channel is enabled for Rx (ENR) */
int stopped_tx; /* Whether channel has been stopped for Tx
@@ -194,8 +194,8 @@ struct ucc_slow_private {
struct list_head confQ; /* frames passed to chip waiting for tx */
u32 first_tx_bd_mask; /* mask is used in Tx routine to save status
and length for first BD in a frame */
- u32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */
- u32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */
+ s32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */
+ s32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */
struct qe_bd *confBd; /* next BD for confirm after Tx */
struct qe_bd *tx_bd; /* next BD for new Tx request */
struct qe_bd *rx_bd; /* next BD to collect after Rx */
--
2.23.0
------------------------------
Subject: Digest Footer
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
------------------------------
End of Linuxppc-dev Digest, Vol 183, Issue 481
**********************************************